[C++20] [Modules] Support generating in-class defined function with try-catch body #129212
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ry-catch body
See the example:
```
export module func;
class C {
public:
void member() try {
} catch (...) {
}
};
```
We woudln't generate the definition for `C::member` but we should. Since
the function is non-inline in modules.
This turns out to be an oversight in parser to me. Since the try-catch
body is relatively rare, so maybe we just forgot it.
ChuanqiXu9
added
clang
llvmbot
added
the
clang:frontend
Member
|
@llvm/pr-subscribers-clang-modules @llvm/pr-subscribers-clang Author: Chuanqi Xu (ChuanqiXu9) ChangesSee the example: We woudln't generate the definition for This turns out to be an oversight in parser to me. Since the try-catch body is relatively rare, so maybe we just forgot it. Full diff: https://github.com/llvm/llvm-project/pull/129212.diff 2 Files Affected:
diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp
index 6c01af55ef3c4..723ebfa59fc03 100644
--- a/clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -632,6 +632,11 @@ void Parser::ParseLexedMethodDef(LexedMethod &LM) {
if (Tok.is(tok::eof) && Tok.getEofData() == LM.D)
ConsumeAnyToken();
+
+ if (auto *FD = dyn_cast_or_null<FunctionDecl>(LM.D))
+ if (isa<CXXMethodDecl>(FD) ||
+ FD->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend))
+ Actions.ActOnFinishInlineFunctionDef(FD);
return;
}
if (Tok.is(tok::colon)) {
diff --git a/clang/test/Modules/try-func-body.cppm b/clang/test/Modules/try-func-body.cppm
new file mode 100644
index 0000000000000..379f5e47f4f8e
--- /dev/null
+++ b/clang/test/Modules/try-func-body.cppm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 %s -fexceptions -fcxx-exceptions -emit-llvm -triple %itanium_abi_triple -o - | FileCheck %s
+
+export module func;
+class C {
+public:
+ void member() try {
+
+ } catch (...) {
+
+ }
+};
+
+// CHECK: define {{.*}}@_ZNW4func1C6memberEv
|
mizvekov
reviewed
Feb 28, 2025
Contributor
mizvekov
left a comment
mizvekov
left a comment
There was a problem hiding this comment.
We could use a scope guard here to avoid the duplication, and this includes the pre-existing EOF handling. This would make this sort of bug more unlikely to happen in the future.
Member
Author
Done |
mizvekov
approved these changes
Mar 4, 2025
Contributor
mizvekov
left a comment
mizvekov
left a comment
There was a problem hiding this comment.
Thanks, LGTM.
With just a little more effort you could add the EOF stuff in there too.
Member
Author
What do you mean by EOF stuff? |
Contributor
It seems you can deduplicate by moving this block into the scope guard: while (Tok.isNot(tok::eof))
ConsumeAnyToken();
if (Tok.is(tok::eof) && Tok.getEofData() == LM.D)
ConsumeAnyToken(); |
Member
Author
Done |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/154/builds/12796 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See the example:
We woudln't generate the definition for
C::memberbut we should. Since the function is non-inline in modules.This turns out to be an oversight in parser to me. Since the try-catch body is relatively rare, so maybe we just forgot it.