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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
