[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.


================
Comment at: clang/lib/AST/Expr.cpp:3164
+      const QualType &QT = cast<DeclRefExpr>(this)->getDecl()->getType();
+      if (QT->isStructureType() && QT.isConstQualified())
+        return true;
----------------
efriedma wrote:
> efriedma wrote:
> > nickdesaulniers wrote:
> > > efriedma wrote:
> > > > nickdesaulniers wrote:
> > > > > nickdesaulniers wrote:
> > > > > > Interesting, playing with this more in godbolt, it looks like the struct doesn't even have to be const qualified.
> > > > > Or, rather, behaves differently between C and C++ mode;
> > > > > 
> > > > > C -> const required
> > > > > C++ -> const not required
> > > > In C++, global variable initializers don't have to be constant expressions at all.
> > > > 
> > > > Do we really need to check GNUMode here? We try to avoid it except for cases where we would otherwise reject valid code.
> > > > 
> > > > Do we need to worry about arrays here?
> > > > In C++, global variable initializers don't have to be constant expressions at all.
> > > 
> > > It looks like my test cases are supported already in Clang today, in C++ mode only and not C.  Maybe there's some alternative code path that I should be looking to reuse?
> > > 
> > > > Do we really need to check GNUMode here?
> > > 
> > > Maybe a `-Wpedantic` diag would be more appropriate otherwise? (GCC does not warn for these cases with `-Wpedantic`.  If the answer to your question is `no`, then that means supporting these regardless of language mode.  (I'm ok with that, was just being maybe overly cautious with `GNUMode`, but maybe folks with better knowledge of the language standards have better thoughts?)
> > > 
> > > > Do we need to worry about arrays here?
> > > 
> > > I don't think arrays are supported: https://godbolt.org/z/RiZPpM
> > Also, do we need to check that we actually have a definition for the variable?
> The C++ standard is substantially different from C.  C++ global initializers can be evaluated at runtime.  So we don't call this code at all in C++.
> 
> Independent of that, we do have pretty complete support for constant evaluation of structs in C++ to support constexpr, and we should be able to leverage that.
> 
> ----
> 
> For arrays, I was thinking of something like this:
> 
> ```
> const int foo[3] = { 0, 1, 2 };
> int bar = foo[0];
> ```
> 
> ----
> 
> We generally don't generate pedantic warnings unless the user uses an extension that's disallowed by the C standard.  (The idea is that clang with -pedantic should generate a diagnostic every place the C standard requires a diagnostic.  It's not a catch-all for extensions.)
> 
> We could separately generate some sort of portability warning, but not sure anyone would care to enable it.
> Do we really need to check GNUMode here?

Will remove.

> Do we need to worry about arrays here?

Yep; as long as the base and index are `const` qualified, GCC allows them.  Will add tests.

> Also, do we need to check that we actually have a definition for the variable?

Yep, will add tests.

> It's not a catch-all for extensions.

Ah, got it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76096/new/

https://reviews.llvm.org/D76096