Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release/19.x: [C++20] [Modules] Treat constexpr/consteval member function as implicitly inline #109076

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

llvmbot
Copy link
Collaborator

@llvmbot llvmbot commented Sep 18, 2024

Backport 74ac96a

Requested by: @ChuanqiXu9

@llvmbot llvmbot added this to the LLVM 19.X Release milestone Sep 18, 2024
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Sep 18, 2024
@llvmbot
Copy link
Collaborator Author

llvmbot commented Sep 18, 2024

@llvm/pr-subscribers-clang

Author: None (llvmbot)

Changes

Backport 74ac96a

Requested by: @ChuanqiXu9


Full diff: https://github.com/llvm/llvm-project/pull/109076.diff

2 Files Affected:

  • (modified) clang/lib/Sema/SemaDecl.cpp (+1)
  • (added) clang/test/Modules/pr107673.cppm (+12)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index d608dd92a4b479..d3c07d0a76f12e 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -9732,6 +9732,7 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
     // the function decl is created above).
     // FIXME: We need a better way to separate C++ standard and clang modules.
     bool ImplicitInlineCXX20 = !getLangOpts().CPlusPlusModules ||
+                               NewFD->isConstexpr() || NewFD->isConsteval() ||
                                !NewFD->getOwningModule() ||
                                NewFD->isFromExplicitGlobalModule() ||
                                NewFD->getOwningModule()->isHeaderLikeModule();
diff --git a/clang/test/Modules/pr107673.cppm b/clang/test/Modules/pr107673.cppm
new file mode 100644
index 00000000000000..dc66c9ac2245b3
--- /dev/null
+++ b/clang/test/Modules/pr107673.cppm
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++20 %s -ast-dump | FileCheck %s
+export module a;
+export class f {
+public:
+    void non_inline_func() {}
+    constexpr void constexpr_func() {}
+    consteval void consteval_func() {}
+};
+
+// CHECK-NOT: non_inline_func {{.*}}implicit-inline
+// CHECK: constexpr_func {{.*}}implicit-inline
+// CHECK: consteval_func {{.*}}implicit-inline

@llvmbot
Copy link
Collaborator Author

llvmbot commented Sep 18, 2024

@llvm/pr-subscribers-clang-modules

Author: None (llvmbot)

Changes

Backport 74ac96a

Requested by: @ChuanqiXu9


Full diff: https://github.com/llvm/llvm-project/pull/109076.diff

2 Files Affected:

  • (modified) clang/lib/Sema/SemaDecl.cpp (+1)
  • (added) clang/test/Modules/pr107673.cppm (+12)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index d608dd92a4b479..d3c07d0a76f12e 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -9732,6 +9732,7 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
     // the function decl is created above).
     // FIXME: We need a better way to separate C++ standard and clang modules.
     bool ImplicitInlineCXX20 = !getLangOpts().CPlusPlusModules ||
+                               NewFD->isConstexpr() || NewFD->isConsteval() ||
                                !NewFD->getOwningModule() ||
                                NewFD->isFromExplicitGlobalModule() ||
                                NewFD->getOwningModule()->isHeaderLikeModule();
diff --git a/clang/test/Modules/pr107673.cppm b/clang/test/Modules/pr107673.cppm
new file mode 100644
index 00000000000000..dc66c9ac2245b3
--- /dev/null
+++ b/clang/test/Modules/pr107673.cppm
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++20 %s -ast-dump | FileCheck %s
+export module a;
+export class f {
+public:
+    void non_inline_func() {}
+    constexpr void constexpr_func() {}
+    consteval void consteval_func() {}
+};
+
+// CHECK-NOT: non_inline_func {{.*}}implicit-inline
+// CHECK: constexpr_func {{.*}}implicit-inline
+// CHECK: consteval_func {{.*}}implicit-inline

@tru
Copy link
Collaborator

tru commented Sep 24, 2024

@ChuanqiXu9 this looks safe enough to be picked. Does the PR look fine to you?

@ChuanqiXu9
Copy link
Member

@ChuanqiXu9 this looks safe enough to be picked. Does the PR look fine to you?

Yes, I'll try to approve it formally.

@ChuanqiXu9 ChuanqiXu9 self-requested a review September 24, 2024 06:15
Copy link
Member

@ChuanqiXu9 ChuanqiXu9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tru tru merged commit b881b16 into llvm:release/19.x Sep 24, 2024
7 of 10 checks passed
Copy link

@ChuanqiXu9 (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

3 participants