aaron.ballman added a comment. In D74130#3079630 <https://reviews.llvm.org/D74130#3079630>, @Tyker wrote:
> In D74130#3073271 <https://reviews.llvm.org/D74130#3073271>, @aaron.ballman > wrote: > >> @Tyker -- are you planning to pick this review back up again sometime in the >> near future? If not, do you care if the review gets commandeered? > > here is an update that i think is close to committable. if it is not i am > fine if it gets commandeered. > > Changes > > - rebased > - fix the default parameter situation for functions and lambdas. > - fix the top level variables not being properly checked. > > the double errors are not fixed on code like: > > consteval int f1() { return 0; } > consteval auto g() { return f1; } > > constexpr auto e = g(); > constexpr auto e1 = f1; Thank you for the update and letting me know you're fine if this is commandeered. FWIW, I am not seeing double errors on that code. Here's the output I get with this patch applied locally: F:\source\llvm-project>cat "C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp" consteval int f1() { return 0; } consteval auto g() { return f1; } constexpr auto e = g(); constexpr auto e1 = f1; F:\source\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only -std=c++2b "C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp" C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:20: error: call to consteval function 'g' is not a constant expression constexpr auto e = g(); ^ C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:20: note: pointer to a consteval declaration is not a constant expression C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:15: note: declared here consteval int f1() { return 0; } ^ C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:16: error: constexpr variable 'e' must be initialized by a constant expression constexpr auto e = g(); ^ ~~~ C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:16: note: pointer to a consteval declaration is not a constant expression C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:15: note: declared here consteval int f1() { return 0; } ^ C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:5:21: error: cannot take address of consteval function 'f1' outside of an immediate invocation constexpr auto e1 = f1; ^ C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:15: note: declared here consteval int f1() { return 0; } ^ C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:5:16: error: constexpr variable 'e1' must be initialized by a constant expression constexpr auto e1 = f1; ^ ~~ C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:5:16: note: pointer to a consteval declaration is not a constant expression C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:15: note: declared here consteval int f1() { return 0; } ^ 4 errors generated. These look like valid errors to me, so am I misunderstanding something? Is the concern that we're emitting `error: constexpr variable '<whatever>' must be initialized by a constant expression` after we already issued a diagnostic on the same line? From my testing, there's more work to be done, but I think it can be done in a follow-up. Consider: template <typename Ty> struct S { consteval S() = default; Ty Val; }; struct foo { foo(); }; S<int> s1; // Correctly rejected S<foo> s2; // Incorrectly accepted With this patch applied, `s1` is correctly diagnosed, but `s2` is not. The reason it's not is because `Sema::CheckForImmediateInvocation()` tests `!Decl->isConsteval()` to early return, and for the instantiation of `S<foo>`, the constructor is marked as "unspecified" rather than `consteval`. The reason for that is because we set `DefaultedDefaultConstructorIsConstexpr` to false at https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/DeclCXX.cpp#L1309 when instantiating the class. I think we may have some confusion where we mean "*IsConstexpr" to mean "is a constexpr function" or "is usable as a constexpr function", because my reading of https://eel.is/c++draft/dcl.constexpr#7 is that the instantiated template constructor IS a constexpr function but it is not usable in a constant expression. However, that might be orthogonal to the fixes here and should be done separately. ================ Comment at: clang/lib/Parse/ParseDecl.cpp:2393 InitScope.pop(); ---------------- Is there a reason we're not moving this one to below `AddInititializerToDecl()` as we did elsewhere? ================ Comment at: clang/lib/Sema/SemaDecl.cpp:14510 + auto LSI = getCurLambda(); + /// If the functcion is not consteval the paramters need to be checked for + /// immediate invocations. ---------------- CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74130/new/ https://reviews.llvm.org/D74130 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
