[cfe-dev] Unneeded declarations invalidation

[Douglas Gregor]

On Jul 7, 2009, at 12:48 AM, Enea Zaffanella wrote:

> Eli Friedman wrote:
>> On Thu, Jul 2, 2009 at 1:30 AM, Abramo  
>> Bagnara<abramobagnara at tin.it> wrote:
>>> The following typescript show what happens when trying to compile a
>>> simple C source not legal for C99 standard, but accepted and  
>>> compiled by
>>> gcc.
>
> [...SNIP...]
>
>>> OTOH in both cases, I'd like to avoid to invalidate the  
>>> declaration and
>>> thus to inhibit ast generation if this is not strictly needed.
>> Mmm... yeah, even if we don't fix this, we probably don't need to  
>> mark
>> the declaration invalid; it's well-formed in any case.
>> -Eli
>
> Hello.
>
> Please find attached a small patch that fixes what is, to my  
> understanding, another occurrence of an unneeded declaration  
> invalidation (triggered by non-standard use of VM types).
>
> The testcase is the following:
> ====================
> void p(int a) {
>  struct {
>    char x[(int)(char*)2 - 4000];
>    char y[a];
>  } f;
> }
> [snip]
>
> Hence, the error is still reported as it was,
> but the AST generation is more faithful to the original.
> The latter property is important for clients such as ours.
>
> Cheers,
> Enea Zaffanella.
> Index: lib/Sema/SemaDecl.cpp
> ===================================================================
> --- lib/Sema/SemaDecl.cpp	(revision 74831)
> +++ lib/Sema/SemaDecl.cpp	(working copy)
> @@ -3962,12 +3962,12 @@
>       Diag(Loc, diag::warn_illegal_constant_array_size);
>       T = FixedTy;
>     } else {
> +      // Do not unnecessarily invalidate declaration
> +      // (the AST is well-formed anyway).
>       if (SizeIsNegative)
>         Diag(Loc, diag::err_typecheck_negative_array_size);
>       else
>         Diag(Loc, diag::err_typecheck_field_variable_size);
> -      T = Context.IntTy;
> -      InvalidDecl = true;
>     }
>   }

The real problem here is that we're replacing the type of the field  
with 'int', when there's no reason to do that. However, I feel that it  
is important to mark this declaration as invalid, because it *is*  
invalid. So, I've committed the change not to replace the type with  
'int', but the declaration is still marked invalid:

	http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20090720/019240.html

Thanks again!

	- Doug