[cfe-dev] Expr::isIntegerConstantExpr() gives bogus results with sizeof()

[Fariborz Jahanian]

I am not sure this is the right forum for reporting LLVM build  
problems. But here it is. After a recent update, I get this build  
error building llvm:

llvm[2]: Compiling Lexer.cpp for Debug build
/Volumes/sandbox/llvm/lib/AsmParser/Lexer.l: In function ‘int  
llvmAsmlex()’:
/Volumes/sandbox/llvm/lib/AsmParser/Lexer.l:278: error: ‘PURE’ was not  
declared in this scope
/Volumes/sandbox/llvm/lib/AsmParser/Lexer.l:279: error: ‘CONST’ was  
not declared in this scope
Lexer.cpp: In function ‘int yy_get_next_buffer()’:
Lexer.cpp:2543: warning: comparison between signed and unsigned  
integer expressions
make[2]: *** [/Volumes/sandbox/llvm/lib/AsmParser/Debug/Lexer.o] Error 1
make[1]: *** [AsmParser/.makeall] Error 2
make: *** [all] Error 1


- Fariborz

On Nov 26, 2007, at 8:40 AM, Ted Kremenek wrote:

>
> On Nov 25, 2007, at 10:06 PM, Chris Lattner wrote:
>
>>>>> So it seems that sizeof() is being computed in bits, but then the
>>>>> operations aren't consistent. (in fact it should be computed in
>>>>> bytes,
>>>>> as that's the value of that operator in C/C++)
>>>>
>>>> Yep, you're right, applied, thanks!
>>>>
>>>> -Chris
>>>
>>> Should the fix instead go in the body of getTypeSize() instead of
>>> within isConstantExpr()?  Should getTypeSize() ever return the
>>> number of bits instead of the number of bytes?  Does this have
>>> something to do with bitfields?
>>
>> This is the right solution.  We want getTypeSize to be unambiguous
>> as to units, and bits are the most constant thing there are.
>> Various strange things like bitfields, non-8-bit-bytes, and
>> datatypes that are not even multiples of byte sizes all conspire to
>> wanting to have a simple and unambiguous unit to represent things
>> in, bits is the right way to go.
>
> Okay.  Seems like a valid reason to me.
>
>> sizeof, on the other hand, is determined in units of sizeof(char).
>> To be fully general, it shouldn't divide by 8, it should divide by
>> TargetInfo.getCharSize().
>
> Should we go ahead and do this?
>
> Nuno:  Thanks again for finding this bug.
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev