[PATCH] D143053: [C++20] [Modules] Pop Expression Evaluation Context when we skip its body during parsing

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a reviewer: cor3ntin.
ChuanqiXu added inline comments.


================
Comment at: clang/lib/Parse/Parser.cpp:1415
+    //
+    // FIXME: It looks not easy to balance PushExpressionEvaluationContext()
+    // and PopExpressionEvaluationContext().
----------------
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?).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143053