Fznamznon marked 2 inline comments as done.
Fznamznon added inline comments.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:15186
+ // different handling.
+ ExprEvalContexts.back().InImmediateFunctionContext = FD->isConsteval();
+
----------------
cor3ntin wrote:
> shafik wrote:
> > I am a bit concerned that we are updating `InImmediateFunctionContext`
> > outside of `PushExpressionEvaluationContext`. So we are now spreading the
> > logic outside of where it was originally contained.
> >
> > I am wondering if instead inside of `PushExpressionEvaluationContext` we
> > could set `InImmediateFunctionContext` like so:
> >
> > ```
> > ExprEvalContexts.back().InImmediateFunctionContext =
> > ExprEvalContexts[ExprEvalContexts.size() - 2]
> > .isImmediateFunctionContext() || NewContext ==
> > ImmediateFunctionContext;
> > ```
> >
> > or something along those lines.
> Here `isImmediateFunctionContext()` will be true, we want it to be false
>
> If we wanted to do somelink like what you are suggesting, we'd need something
> like a new `EnteringFunctionDefinitionContext` boolean parameter to
> PushExpressionEvaluationContext. so that
> InImmediateFunctionContext is only set to the outer context value when
> `EnteringFunctionDefinitionContext`would be false
>
> Given that we should only get in that situation from
> `ActOnStartOfFunctionDef`, I'm not sure that would be a better design
Thanks, @cor3ntin .
This is the exact reason why I implemented the fix this way.
================
Comment at: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp:120
+
+namespace GH61142 {
+
----------------
aaron.ballman wrote:
> Would it make sense to add some CHECK lines to check the actual codegen is
> sensible?
Well, the assertion was due to consteval function reaching codegen, so I added
checks that it doesn't appear in the IR in any way.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147531/new/
https://reviews.llvm.org/D147531
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
