cor3ntin accepted this revision. cor3ntin added inline comments. This revision is now accepted and ready to land.
================ Comment at: clang/lib/Parse/Parser.cpp:1415 + // + // FIXME: It looks not easy to balance PushExpressionEvaluationContext() + // and PopExpressionEvaluationContext(). ---------------- ChuanqiXu wrote: > cor3ntin wrote: > > shafik wrote: > > > It does seem a bit ad-hoc > > could we use `ExitFunctionBodyRAII` here, maybe ? It already deals with > > lambdas > > It does seem a bit ad-hoc > > I agree. I feel the current explicit style for pushing and popping different > contexts looks not easy to maintain... I just want to prevent the crash for > now and I feel the patch wouldn't be a burden when someday we want to > refactor this. > > > could we use ExitFunctionBodyRAII here, maybe ? It already deals with > > lambdas > > Maybe. But it wouldn't be simpler. We need to move `ExitFunctionBodyRAII` and > we need to write: > > ``` > ExitFunctionBodyRAII ExitRAII(Actions, > isLambdaCallOperator(dyn_cast_if_present<FunctionDecl>(Res))); > return Res; > ``` > > which looks odd and unnecessary. > > And if you're saying something like: > > ``` > Decl *Res = Actions.ActOnStartOfFunctionDef(getCurScope(), D, > TemplateInfo.TemplateParams > ? > *TemplateInfo.TemplateParams > : MultiTemplateParamsArg(), > &SkipBody, BodyKind); > > > ExitFunctionBodyRAII ExitRAII(Actions, > isLambdaCallOperator(dyn_cast_if_present<FunctionDecl>(Res))); > ``` > > and remove the ExitFunctionBodyRAII in `ActOnFinishFunctionBody`. I guess it > can't work in this way simply. Since the comment > (https://github.com/llvm/llvm-project/blob/c6795b1d37cee586d9b98dade64432f8f6bd004b/clang/lib/Sema/SemaDecl.cpp#L15811-L15818) > says we need to pop expression evaluation context before popping decl > context (and function scope info?). I think you are right, your change is probably as good as anything without doing a major refactor. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143053/new/ https://reviews.llvm.org/D143053 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
