efriedma 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;
----------------
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?


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

Reply via email to