[PATCH] D53921: Compound literals, global array decls, and enums require constant inits

[Bill Wendling via Phabricator via cfe-commits]

void marked 3 inline comments as done.
void added inline comments.
Herald added a reviewer: shafik.


================
Comment at: lib/Sema/SemaDecl.cpp:16086-16094
           CheckConvertedConstantExpression(Val, EltTy, EnumVal,
                                            CCEK_Enumerator);
         if (Converted.isInvalid())
           Val = nullptr;
         else
           Val = Converted.get();
       } else if (!Val->isValueDependent() &&
----------------
rsmith wrote:
> I think it would make sense to create the `ConstantExpr` wrapper node in `CheckConvertedConstantExpr` / `VerifyIntegerConstantExpr`. (That's also where it'd make sense to cache the evaluated value on the wrapper node once we start doing so.)
I thought the purpose of `ConstantExpr` is to specify those places where a constant expression is required. I.e., we can't have something like:

```
int z;
foo y = (foo){z + 2};
```

In this case, `z + 2` would be wrapped by the `ConstantExpr` class. But in a function or module scope, then it would be fine:

```
void x(int z) {
  foo y = (foo){z + 2};
}
```

So `z + 2` wouldn't be wrapped. If I perform the wrapping in `CheckConvertedConstantExpr`, et al, then it doesn't seem like I have the context to say that it's a) a compound literal, and b) in file scope. So how can I correctly wrap it?


================
Comment at: lib/Sema/SemaExpr.cpp:5752
         !literalType->isDependentType()) // C99 6.5.2.5p3
       if (CheckForConstantInitializer(LiteralExpr, literalType))
         return ExprError();
----------------
rsmith wrote:
> Can we create the `ConstantExpr` in here instead (assuming we need it at all)?
> 
> (The reason it's not clear to me that we need it at all is that file-scope compound literals can only appear in contexts where a constant expression is required *anyway* -- be they in array bounds, enumerators, or the initializer of a global variable.)
I changed the code to wrap around here. But see my comment above. I think we might be thinking about `ConstantExpr` differently.


Repository:
  rC Clang

https://reviews.llvm.org/D53921