[clang] 10d85dc - [C++20] [Modules] Pop Expression Evaluation Context when we skip its body during parsing

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-02-03T10:27:02+08:00
New Revision: 10d85dc75468074d2aceb1a648125167128d6f25

URL: https://github.com/llvm/llvm-project/commit/10d85dc75468074d2aceb1a648125167128d6f25
DIFF: https://github.com/llvm/llvm-project/commit/10d85dc75468074d2aceb1a648125167128d6f25.diff

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

Close https://github.com/llvm/llvm-project/issues/60275

The root cause of issue 60275 is the imbalance of
PushExpressionEvaluationContext() and PopExpressionEvaluationContext().

See
https://github.com/llvm/llvm-project/blob/f1c4f927f7c15b5efdc3589c050fd0513bf6b303/clang/lib/Parse/Parser.cpp#L1396-L1437

We will PushExpressionEvaluationContext() in ActOnStartOfFunctionDef()
in line 1396 and we should pop it in ActOnFinishFunctionBody later.
However if we skip the function body in line 1402, the expression
evaluation context will not be popped. Then here is the issue report. I
fix the issue by inserting codes to pop the expression evaluation
context explicitly if the function body is skipped. Maybe this looks
like an ad-hoc fix. But if we want to fix this in a pretty way, we
should refactor the current framework for pushing and popping expression
evaluation contexts. Currently there are 23
PushExpressionEvaluationContext() callsities and 21
PopExpressionEvaluationContext() callsites in the code. And it seems not
easy to balance them well and fast. So I suggest to land this fix first.
At least it can prevent the crash.

Reviewed By: cor3ntin

Differential Revision: https://reviews.llvm.org/D143053

Added: 
    clang/test/Modules/pr60275.cppm

Modified: 
    clang/lib/Parse/Parser.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 6db3dc3156fdf..c197d6bdde672 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/ASTLambda.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
@@ -1404,6 +1405,17 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D,
     if (BodyKind == Sema::FnBodyKind::Other)
       SkipFunctionBody();
 
+    // ExpressionEvaluationContext is pushed in ActOnStartOfFunctionDef
+    // and it would be popped in ActOnFinishFunctionBody.
+    // We pop it explcitly here since ActOnFinishFunctionBody won't get called.
+    //
+    // Do not call PopExpressionEvaluationContext() if it is a lambda because
+    // one is already popped when finishing the lambda in BuildLambdaExpr().
+    //
+    // FIXME: It looks not easy to balance PushExpressionEvaluationContext()
+    // and PopExpressionEvaluationContext().
+    if (!isLambdaCallOperator(dyn_cast_if_present<FunctionDecl>(Res)))
+      Actions.PopExpressionEvaluationContext();
     return Res;
   }
 

diff  --git a/clang/test/Modules/pr60275.cppm b/clang/test/Modules/pr60275.cppm
new file mode 100644
index 0000000000000..57b31c6952bea
--- /dev/null
+++ b/clang/test/Modules/pr60275.cppm
@@ -0,0 +1,40 @@
+// Address: https://github.com/llvm/llvm-project/issues/60275
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -emit-module-interface %t/a.cppm -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cpp -fmodule-file=%t/a.pcm -emit-llvm -o - | FileCheck %t/b.cpp
+//--- foo.h
+
+consteval void global() {}
+
+//--- a.cppm
+module;
+#include "foo.h"
+export module a;
+
+//--- b.cpp
+#include "foo.h"
+import a;
+
+consteval int b() {
+	return 0;
+}
+
+struct bb {
+	int m = b();
+};
+
+void bbb() {
+	bb x;
+}
+
+// CHECK: define{{.*}}_ZN2bbC2Ev({{.*}}[[THIS:%.+]])
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   [[THIS_ADDR:%.*]] = alloca ptr
+// CHECK-NEXT:   store ptr [[THIS]], ptr [[THIS_ADDR]]
+// CHECK-NEXT:   [[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]]
+// CHECK-NEXT:   [[M_ADDR:%.*]] = getelementptr{{.*}}%struct.bb, ptr [[THIS1]], i32 0, i32 0
+// CHECK-NEXT:   store i32 0, ptr [[M_ADDR]]