[PATCH] D151094: [clang] Implement P2564 "consteval must propagate up"

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5022
+                                                    SourceLocation Loc) {
+  assert(FD->isImmediateEscalating());
+
----------------
aaron.ballman wrote:
> I think I'm a bit surprised to see this assert in a function named `CheckIf` -- I would assume that we'd return false in this case?
I think we could but checking whether a non-escalating function is immediate does not make sense to me


================
Comment at: clang/lib/Serialization/ASTWriterStmt.cpp:629
+      !E->refersToEnclosingVariableOrCapture() && !E->isNonOdrUse() &&
+      !E->isImmediateEscalating()) {
     AbbrevToUse = Writer.getDeclRefExprAbbrev();
----------------
aaron.ballman wrote:
> Can you explain why this checks that the expression is not immediate escalating? (What test case exercises this?)
Not entirely. My understanding is that we can only use an abbreviation when it's just a reference to a Decl with no additional non-default property, otherwise it needs to be fully serialized (otherwise the "immediate escalating" bit would be lost and deserIAlizing incorrect 


================
Comment at: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp:20
 consteval X g() { return {0}; }
-void f() { g(); }
+void f() { (void)g(); }
 
----------------
aaron.ballman wrote:
> Why is the cast to `void` added?
oups, i wasn't supposed to commit that!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151094