[cfe-commits] r63242 - in /cfe/trunk: Driver/RewriteObjC.cpp include/clang/AST/Decl.h include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/clang/Basic/DiagnosticSemaKinds.def lib/AST/Expr.cpp lib/AST/StmtPrinter.cpp lib/CodeGen/CGExprAgg.

[Eli Friedman]

On Wed, Jan 28, 2009 at 1:54 PM, Douglas Gregor <dgregor at apple.com> wrote:
> +      // Implicitly-generated value initializations are okay.
> +      if (isa<CXXZeroInitValueExpr>(Exp->getInit(i)) &&
> +          cast<CXXZeroInitValueExpr>(Exp->getInit(i))->isImplicit())
> +        continue;
> +
>       if (CheckForConstantInitializer(Exp->getInit(i),
>                                       Exp->getInit(i)->getType()))
>         return true;

This doesn't look quite right... I'm pretty sure a
CXXZeroInitValueExpr should be a valid constant initializer even if it
isn't in an init list.

In any case, this seems like a massive abuse of CXXZeroInitValueExpr,
and I'm not completely sure that the C++ rules match what you want
here.  I'd suggest a new expression type to represent the holes.

> +      SemaRef->Diag(D->getStartLocation(),
> +                    diag::warn_designator_into_union_broken_init)
> +        << SourceRange(D->getStartLocation(), DIE->getSourceRange().getEnd());

If we're going to do the wrong thing, this should be an error.

> +    SemaRef->Diag(D->getEllipsisLoc(),
> +                  diag::warn_gnu_array_range_designator_unsupported)
> +      << SourceRange(D->getLBracketLoc(), D->getRBracketLoc());

If we're going to do the wrong thing, this should be an error.

> +    // Here, xs[0].a == 0 and xs[0].b == 3, since the second,
> +    // designated initializer re-initializes the whole
> +    // subobject [0], overwriting previous initializers.
> +    SemaRef->Diag(InitRange.getBegin(), diag::warn_subobject_initializer_overrides)
> +      << InitRange;
> +    SemaRef->Diag(ExistingInit->getSourceRange().getBegin(),
> +                  diag::note_previous_initializer)
> +      << ExistingInit->hasSideEffects(SemaRef->Context)
> +      << ExistingInit->getSourceRange();

There's one somewhat dangerous case here; take the following example
at file scope:

struct X { int a, b; };
struct X xs[] = { [0] = (struct X) { 1, 2 }, [0].b = 3 };

In gnu89 mode in gcc, the init list is actually equivalent to "{1,
3}".  clang, on the other hand, will give different results (although
it will also warn, so it might not be so bad).

> +bool Expr::hasSideEffects(ASTContext &Ctx) const {
> +  EvalResult Result;
> +  Evaluate(Result, Ctx);
> +  return Result.HasSideEffects;
> +}

I don't think this will work quite correctly; you probably want
something like "return !Evaluate(Result, Ctx) ||
Result.HasSideEffects"

In general, though, this does seem like a good approach.

-Eli