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

[Eli Friedman]

On Tue, Jun 3, 2008 at 10:54 AM, Steve Naroff <snaroff at apple.com> wrote:
>
> 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?

I already replied to this.

>> 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...

I haven't actually tried anything with this change, so I really have
no idea what it might practically break.  However, issue with not
computing the actual size is likely to break *your* code if you
actually tried to run it through CodeGen.

-Eli