[cfe-dev] Unravelling i-c-e + the gnu mess

[Eli Friedman]

On Wed, Jul 9, 2008 at 11:05 AM, Chris Lattner <clattner at apple.com> wrote:
> Ok, I'm preparing to do battle with Expr::isIntegerConstantExpr and
> want to consult the guru's to see if this is a sane plan. :)
>
> Problem statement:
>   1) we want to support fully conformant C code with GNU extensions
> disabled.
>   2) We want to support GNU extensions, but emit ext-warns when they
> are used.
>   3) If a subexpr is a GNU ice, we don't want to emit warnings
> multiple times in the subexpr.  The canonical horrible example would
> be test/Sema/darwin-align-cast.c

I agree so far.

> 2. Change Expr::isIntegerConstantExpr to be something like this:
>
>   // See if this expr is a real i-c-e.
>   unsigned Diag = isIntegerConstExprHelper(this, ..., false)
>   if (Diag == 0) return true;
>
>   // See if it is a gnu one.
>   if (!LangOptions.NoExtensions) {
>     if (isIntegerConstExprHelper(this, ..., false) == 0) {
>       Emit Warning about using GNU i-c-e.
>       return true;
>     }
>   }
>   return false;

This is where it get really messy... the issue is that saying that
something is an integer constant expression has indirect effects in
some cases.  Non-ICE array bounds indicate a VLA, and the
compatibility rules for those are different from those for constant
arrays.  A conditional expression with a null pointer constant can
silently end up with a different type from an conditional with a
pointer that happens to evaluate to zero.  These are edge cases, but
-pedantic shouldn't affect semantics.

I think the right approach is what we're doing in
Sema::TryFixInvalidVariablyModifiedType: if we run into a situation
where we print a warning/error, but gcc accepts the code, try to
recover at the point where we would print the diagnostic.

If it's convenient, we could add a "RequireIntegerConstantExpr" method
which does roughly "if (Expr->ICE()) return val; if
(Expr->TryEvaluate()) {pedwarn(); return val;} error(); return
failure;".

Downsides to this approach: it's possible it will let stuff slip by
without a warning in non-pedantic mode that gcc wouldn't accept, and
this will leave constructs like the glibc tgmath.h broken.

On a related note, I'm considering rewriting
Expr::isIntegerConstantExpression() to delegate actual calculation to
Expr::TryEvaluate() at some point, to reduce code duplication.

> 3. Change Expr::isIntegerConstantExpr to return the diag reason up to
> its callers so they can emit more specific diags in some cases.

We should definitely do this eventually in any case.

-Eli