[cfe-commits] r51907 - in /cfe/trunk: lib/AST/Expr.cpp test/Sema/gcc-cast-ext.m

[Steve Naroff]

On Jun 3, 2008, at 10:43 AM, Eli Friedman wrote:

> On Tue, Jun 3, 2008 at 10:15 AM, Steve Naroff <snaroff at apple.com>  
> wrote:
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
>> --- cfe/trunk/lib/AST/Expr.cpp (original)
>> +++ cfe/trunk/lib/AST/Expr.cpp Tue Jun  3 12:15:29 2008
>> @@ -918,6 +918,11 @@
>>    if (!SubExpr->getType()->isArithmeticType() ||
>>        !getType()->isIntegerType()) {
>>      if (Loc) *Loc = SubExpr->getLocStart();
>> +      // GCC accepts pointers as an extension.
>> +      // FIXME: check getLangOptions().NoExtensions. At the  
>> moment, it doesn't
>> +      // appear possible to get langOptions() from the Expr.
>> +      if (SubExpr->getType()->isPointerType()) // && !NoExtensions
>> +        return true;
>>      return false;
>>    }
>
> This is no good: this potentially breaks standards-compliant code (as
> I said in reply to the mail you sent to cfe-dev), and it's completely
> impossible to warn for illegal cases.

If Expr had access to langOptions(), why is this impossible to warn  
about?

> If you really need this
> specific construct to work, we can add a special-case to the struct
> decl code to allow it.
>
> Also, your implementation is completely broken; it treats *all*
> pointers as integer constant expressions, and it doesn't actually
> bother to compute the correct value.  This will silently break code
> that uses this construct; if I'm not mistaken, sizeof(struct xx) in
> the testcase you added is zero.
>

At the moment, it's more important for this to "work" on GCC dependent  
code that we are trying to compile (rather than worry too much about  
hypothetical cases).

If this change negatively effects *any* current code being worked on,  
I am happy to back it out...

Pragmatically yours,

snaroff

> -Eli