cor3ntin added inline comments.
================ Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1905 + if (SemaRef.LangOpts.CPlusPlus2b) { + if (!VD->getType()->isLiteralType(SemaRef.Context)) + SemaRef.Diag(VD->getLocation(), ---------------- aaron.ballman wrote: > aaron.ballman wrote: > > cor3ntin wrote: > > > hubert.reinterpretcast wrote: > > > > This seems to trigger even when the type is dependent: > > > > ``` > > > > <stdin>:1:36: warning: definition of a variable of non-literal type in > > > > a constexpr function is incompatible with C++ standards before C++2b > > > > [-Wpre-c++2b-compat] > > > > auto qq = [](auto x) { decltype(x) n; }; > > > > ^ > > > > 1 warning generated. > > > > ``` > > > > > > > > This also seems to emit even when `Kind` is not > > > > `Sema::CheckConstexprKind::Diagnose` (unlike the > > > > `static`/`thread_local` case above). Is the `CheckLiteralType` logic > > > > not reusable for this? > > > You are right, thanks for noticing that, it was rather bogus. > > > The reason I'm not using CheckLiteralType is to avoid duplicating a > > > diagnostics message, as CheckLiteralType doesn't allow us to pass > > > parameter to the diagnostic message. > > > > > > It leaves us with an uncovered scenario though: We do not emit the > > > warning on template instantiation, and I don't think there is an easy > > > way to do that. > > > The reason I'm not using CheckLiteralType is to avoid duplicating a > > > diagnostics message, as CheckLiteralType doesn't allow us to pass > > > parameter to the diagnostic message. > > > > Huh? > > > > ``` > > static bool CheckLiteralType(Sema &SemaRef, Sema::CheckConstexprKind Kind, > > SourceLocation Loc, QualType T, unsigned > > DiagID, > > Ts &&...DiagArgs) { > > ... > > } > > ``` > > I would hope `DiagArgs` should do exactly that? :-) > > It leaves us with an uncovered scenario though: We do not emit the warning > > on template instantiation, and I don't think there is an easy way to do > > that. > > I believe the code paths that lead us here all come from > `Sema::CheckConstexprFunctionDefinition()` which is called from > `Sema::ActOnFinishFunctionBody()` which seems to be called when instantiating > templates in `Sema::InstantiateFunctionDefinition()`, so perhaps some more > investigation is needed as to why we're not reaching this for template > instantiations. We could add something in addition of `Sema::CheckConstexprKind::CheckValid` and `Sema::CheckConstexprKind::Diagnose`, but * not for implicit lambdas, because we should not warn on lambdas that won't be constexpr * for explicit constexpr lambdas / functions, it would force us to call CheckConstexprFunctionDefinition on instanciation, which we currently don't do, and is not free for that one warning - and we would have to not-reemit the other warnings. It seems like quite a fair amount of work for a diagnostic not enabled by default. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122249/new/ https://reviews.llvm.org/D122249 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits