llvm dev - Jul 2004 - [LLVMdev] Add assert in llvm::StroreInst::init and llvm::LoadInst::init

I'm make silly error (swap arguments in llvm::StroreInst constructor call:

llvm::Value* var = genExpr(bb,*varExpr,false);
llvm::Value* val = genExpr(bb,*valExpr,true );
llvm::StoreInst* lStore = new llvm::StoreInst(var,val,bb);

instead

assert(var && var->getType()->getTypeID()==llvm::Type::PointerTyID
&& "var
side isn't pointer type");
llvm::StoreInst* lStore = new llvm::StoreInst(val,var,bb);

For val with type T* and var with type T** error catched in verifier:

Stored value type does not match pointer operand type!
        store "Type10::Class1"** %result, "Type10::Class1"*
%1
 "Type10::Class1"Broken module found, compilation aborted!


But if val type isn't pointer type error catched only deep in verifier code
with non helpful message:

Assertion failed: (isa<X>(Val) && "cast<Ty>() argument
of incompatible
type!"), function cast, file
/home/wanderer/pkg/build/llvm/src/llvm/include/Support/Casting.h, line 197.

I propose add assert

assert(Ptr && Ptr->getType()->getTypeID()==llvm::Type::PointerTyID
&& "Ptr
must have pointer type.")

in

llvm::StroreInst::init and llvm::LoadInst::init functions

This is requared including llvm/Type.h or moving init function definitions
to iMemory.cpp.

I don't known more acceptable variant and then sending as attachment both
versions: iMemory.patch1 and iMemory.patch2 .

Vladimir
-------------- next part --------------
A non-text attachment was scrubbed...
Name: iMemory.patch2
Type: application/octet-stream
Size: 2072 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20040701/3dba11c4/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: iMemory.patch1
Type: application/octet-stream
Size: 897 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20040701/3dba11c4/attachment-0001.obj>

On Thu, 1 Jul 2004, Vladimir Merzliakov wrote:
> I propose add assert
>
> assert(Ptr &&
Ptr->getType()->getTypeID()==llvm::Type::PointerTyID && "Ptr
> must have pointer type.")
> in
> llvm::StroreInst::init and llvm::LoadInst::init functions
Sounds good.  One comment, FYI.  The above can be written to use:
  ... isa<PointerType>(Ptr->getType()) ...

instead of digging in with getTypeID().  This makes the code a bit easier
to read.
> This is requared including llvm/Type.h or moving init function definitions
> to iMemory.cpp.
>
> I don't known more acceptable variant and then sending as attachment
both
> versions: iMemory.patch1 and iMemory.patch2 .
Sounds great!  I've taken the second approach (to avoid including Type.h
into iMemory.h, and applied a slightly modified version of your patch:

http://mail.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20040628/015627.html
http://mail.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20040628/015628.html

Thanks a lot!

-Chris

-- 
http://llvm.cs.uiuc.edu/
http://www.nondot.org/~sabre/Projects/