[clang] [C++20][Modules] Restore inliness of constexpr/consteval functions defined in-class (PR #109470)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Sep 21 01:09:01 PDT 2024
https://github.com/tomasz-kaminski-sonarsource updated https://github.com/llvm/llvm-project/pull/109470
>From 6ff9964b7180cc9279c2742b14f69cc966a027a1 Mon Sep 17 00:00:00 2001
From: Tomasz Kaminski <tomasz.kaminski at sonarsource.com>
Date: Fri, 20 Sep 2024 17:21:33 +0200
Subject: [PATCH 1/2] [C++20][Modules] Restore inliness of constexpr/consteval
functions defined in-class
This correct issue, when the functions declared as `constexpr` are `consteval`,
are not considered to be inline (`isInlined()` is false) when defined inside class
attached to named module:
```c++
export module mod;
struct Clazz {
constexpr void f1() { } // non-inline
constexpr void f2();
friend constexpr void f3() {} // non-inline
};
constexpr void Clazz::f3() {} // inline
```
This conflicts with [decl.constexpr] p1:
> A function or static data member declared with the constexpr or consteval
specifier on its first declaration is implicitly an inline function or
variable ([dcl.inline]).
If any declaration of a function or function template has a constexpr or
consteval specifier, then all its declarations shall contain the same
specifier/)
This regression was introduced by https://github.com/llvm/llvm-project/commit/97af17c5,
where the inline of such function was accidentally removed
The corresponding wording in [class.friend] and p6 [class.mfct] p1 uses "if" and not "if and only if",
thus does not imply that these are only cases where such functions are inline.
---
clang/lib/Sema/SemaDecl.cpp | 14 +++----
.../test/CXX/class/class.friend/p7-cxx20.cpp | 38 ++++++++++++++++---
clang/test/CXX/class/class.mfct/p1-cxx20.cpp | 30 +++++++++++++--
3 files changed, 67 insertions(+), 15 deletions(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index de8805e15bc750..0ea99c43037e5e 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -9760,8 +9760,8 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
if (getLangOpts().CPlusPlus) {
// The rules for implicit inlines changed in C++20 for methods and friends
// with an in-class definition (when such a definition is not attached to
- // the global module). User-specified 'inline' overrides this (set when
- // the function decl is created above).
+ // the global module). This does not affect declarations, that are already
+ // inline, for example due being declared `inline` or `consteval`
// FIXME: We need a better way to separate C++ standard and clang modules.
bool ImplicitInlineCXX20 = !getLangOpts().CPlusPlusModules ||
NewFD->isConstexpr() || NewFD->isConsteval() ||
@@ -9772,14 +9772,14 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
bool isVirtual = D.getDeclSpec().isVirtualSpecified();
bool hasExplicit = D.getDeclSpec().hasExplicitSpecifier();
isFriend = D.getDeclSpec().isFriendSpecified();
- if (isFriend && !isInline && D.isFunctionDefinition()) {
+ if (ImplicitInlineCXX20 && isFriend && D.isFunctionDefinition()) {
// Pre-C++20 [class.friend]p5
// A function can be defined in a friend declaration of a
// class . . . . Such a function is implicitly inline.
// Post C++20 [class.friend]p7
// Such a function is implicitly an inline function if it is attached
// to the global module.
- NewFD->setImplicitlyInline(ImplicitInlineCXX20);
+ NewFD->setImplicitlyInline();
}
// If this is a method defined in an __interface, and is not a constructor
@@ -10083,15 +10083,15 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
break;
}
- if (isa<CXXMethodDecl>(NewFD) && DC == CurContext &&
- D.isFunctionDefinition() && !isInline) {
+ if (ImplicitInlineCXX20 && isa<CXXMethodDecl>(NewFD) && DC == CurContext &&
+ D.isFunctionDefinition()) {
// Pre C++20 [class.mfct]p2:
// A member function may be defined (8.4) in its class definition, in
// which case it is an inline member function (7.1.2)
// Post C++20 [class.mfct]p1:
// If a member function is attached to the global module and is defined
// in its class definition, it is inline.
- NewFD->setImplicitlyInline(ImplicitInlineCXX20);
+ NewFD->setImplicitlyInline();
}
if (!isFriend && SC != SC_None) {
diff --git a/clang/test/CXX/class/class.friend/p7-cxx20.cpp b/clang/test/CXX/class/class.friend/p7-cxx20.cpp
index 8843d55910ea2d..0ce77b353c2499 100644
--- a/clang/test/CXX/class/class.friend/p7-cxx20.cpp
+++ b/clang/test/CXX/class/class.friend/p7-cxx20.cpp
@@ -46,14 +46,42 @@ module;
export module M;
class Z {
- friend void z(){};
+ friend void z1(){};
};
+
+class Inline {
+ friend inline void z2(){};
+};
+
+class Constexpr {
+ friend constexpr void z3(){};
+};
+
+class Consteval {
+ friend consteval void z4(){};
+};
+
// CHECK-MOD: |-CXXRecordDecl {{.*}} <.{{/|\\\\?}}header.h:2:1, line:4:1> line:2:7 in M.<global> hidden class A definition
// CHECK-MOD: | |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M.<global> hidden implicit class A
// CHECK-MOD-NEXT: | `-FriendDecl {{.*}} <line:3:3, col:19> col:15 in M.<global>
// CHECK-MOD-NEXT: | `-FunctionDecl {{.*}} parent {{.*}} <col:3, col:19> col:15 in M.<global> hidden friend_undeclared a 'void ()' implicit-inline
-// CHECK-MOD: `-CXXRecordDecl {{.*}} <module.cpp:6:1, line:8:1> line:6:7 in M hidden class Z{{( ReachableWhenImported)?}} definition
-// CHECK-MOD: |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M hidden implicit class Z{{( ReachableWhenImported)?}}
-// CHECK-MOD-NEXT: `-FriendDecl {{.*}} <line:7:3, col:19> col:15 in M{{( ReachableWhenImported)?}}
-// CHECK-MOD-NEXT: `-FunctionDecl {{.*}} parent {{.*}} <col:3, col:19> col:15 in M hidden friend_undeclared z 'void ()'{{( ReachableWhenImported)?}}
+// CHECK-MOD: |-CXXRecordDecl {{.*}} <module.cpp:6:1, line:8:1> line:6:7 in M hidden class Z{{( ReachableWhenImported)?}} definition
+// CHECK-MOD: | |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M hidden implicit class Z{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: | `-FriendDecl {{.*}} <line:7:3, col:20> col:15 in M{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: | `-FunctionDecl {{.*}} parent {{.*}} <col:3, col:20> col:15 in M hidden friend_undeclared z1 'void ()'{{( ReachableWhenImported)?}}
+
+// CHECK-MOD: |-CXXRecordDecl {{.*}} <line:10:1, line:12:1> line:10:7 in M hidden class Inline{{( ReachableWhenImported)?}} definition
+// CHECK-MOD: | |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M hidden implicit class Inline{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: | `-FriendDecl {{.*}} <line:11:3, col:27> col:22 in M{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: | `-FunctionDecl {{.*}} parent {{.*}} <col:3, col:27> col:22 in M hidden friend_undeclared z2 'void ()'{{( ReachableWhenImported)?}} inline
+
+// CHECK-MOD: |-CXXRecordDecl {{.*}} <line:14:1, line:16:1> line:14:7 in M hidden class Constexpr{{( ReachableWhenImported)?}} definition
+// CHECK-MOD: | |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M hidden implicit class Constexpr{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: | `-FriendDecl {{.*}} <line:15:3, col:30> col:25 in M{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: | `-FunctionDecl {{.*}} parent {{.*}} <col:3, col:30> col:25 in M hidden constexpr friend_undeclared z3 'void ()'{{( ReachableWhenImported)?}} implicit-inline
+
+// CHECK-MOD: `-CXXRecordDecl {{.*}} <line:18:1, line:20:1> line:18:7 in M hidden class Consteval{{( ReachableWhenImported)?}} definition
+// CHECK-MOD: |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M hidden implicit class Consteval{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: `-FriendDecl {{.*}} <line:19:3, col:30> col:25 in M{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: `-FunctionDecl {{.*}} parent {{.*}} <col:3, col:30> col:25 in M hidden consteval friend_undeclared z4 'void ()'{{( ReachableWhenImported)?}} implicit-inline
diff --git a/clang/test/CXX/class/class.mfct/p1-cxx20.cpp b/clang/test/CXX/class/class.mfct/p1-cxx20.cpp
index 5b24668d7b6617..9b990e13c09791 100644
--- a/clang/test/CXX/class/class.mfct/p1-cxx20.cpp
+++ b/clang/test/CXX/class/class.mfct/p1-cxx20.cpp
@@ -48,10 +48,34 @@ class Z {
void z(){};
};
+class Inline {
+ inline void z(){};
+};
+
+class Constexpr {
+ constexpr void z(){};
+};
+
+class Consteval {
+ consteval void z(){};
+};
+
// CHECK-MOD: |-CXXRecordDecl {{.*}} <.{{/|\\\\?}}header.h:2:1, line:4:1> line:2:7 in M.<global> hidden class A definition
// CHECK-MOD: | |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M.<global> hidden implicit class A
// CHECK-MOD-NEXT: | `-CXXMethodDecl {{.*}} <line:3:3, col:12> col:8 in M.<global> hidden a 'void ()' implicit-inline
-// CHECK-MOD: `-CXXRecordDecl {{.*}} <module.cpp:6:1, line:8:1> line:6:7 in M hidden class Z{{( ReachableWhenImported)?}} definition
-// CHECK-MOD: |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M hidden implicit class Z{{( ReachableWhenImported)?}}
-// CHECK-MOD-NEXT: `-CXXMethodDecl {{.*}} <line:7:3, col:12> col:8 in M hidden z 'void ()'{{( ReachableWhenImported)?}}
+// CHECK-MOD: |-CXXRecordDecl {{.*}} <module.cpp:6:1, line:8:1> line:6:7 in M hidden class Z{{( ReachableWhenImported)?}} definition
+// CHECK-MOD: | |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M hidden implicit class Z{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: | `-CXXMethodDecl {{.*}} <line:7:3, col:12> col:8 in M hidden z 'void ()'{{( ReachableWhenImported)?}}
+
+// CHECK-MOD: |-CXXRecordDecl {{.*}} <line:10:1, line:12:1> line:10:7 in M hidden class Inline{{( ReachableWhenImported)?}} definition
+// CHECK-MOD: | |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M hidden implicit class Inline{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: | `-CXXMethodDecl {{.*}} <line:11:3, col:19> col:15 in M hidden z 'void ()'{{( ReachableWhenImported)?}} inline
+
+// CHECK-MOD: |-CXXRecordDecl {{.*}} <line:14:1, line:16:1> line:14:7 in M hidden class Constexpr{{( ReachableWhenImported)?}} definition
+// CHECK-MOD: | |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M hidden implicit class Constexpr{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: | `-CXXMethodDecl {{.*}} <line:15:3, col:22> col:18 in M hidden constexpr z 'void ()'{{( ReachableWhenImported)?}} implicit-inline
+
+// CHECK-MOD: `-CXXRecordDecl {{.*}} <line:18:1, line:20:1> line:18:7 in M hidden class Consteval{{( ReachableWhenImported)?}} definition
+// CHECK-MOD: |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M hidden implicit class Consteval{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: `-CXXMethodDecl {{.*}} <line:19:3, col:22> col:18 in M hidden consteval z 'void ()'{{( ReachableWhenImported)?}} implicit-inline
>From 215b045ac61883a9802f7a572075588dd853ec17 Mon Sep 17 00:00:00 2001
From: Tomasz Kaminski <tomasz.kaminski at sonarsource.com>
Date: Sat, 21 Sep 2024 10:05:08 +0200
Subject: [PATCH 2/2] Updated comment and docs
---
clang/docs/ReleaseNotes.rst | 2 ++
clang/lib/Sema/SemaDecl.cpp | 5 +++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f4535db7356194..5d014f729252cb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -407,6 +407,8 @@ Bug Fixes to C++ Support
- Clang now uses the correct set of template argument lists when comparing the constraints of
out-of-line definitions and member templates explicitly specialized for a given implicit instantiation of
a class template. (#GH102320)
+- Clang now correctly considers member functions with in-class definition of `constexpr` and `consteval` to be inline,
+ when the class is attached to named module.
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 0ea99c43037e5e..7b7b1ef449613d 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -9760,8 +9760,9 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
if (getLangOpts().CPlusPlus) {
// The rules for implicit inlines changed in C++20 for methods and friends
// with an in-class definition (when such a definition is not attached to
- // the global module). This does not affect declarations, that are already
- // inline, for example due being declared `inline` or `consteval`
+ // the global module). This does not affect declarations that are already
+ // inline (whether explicitly or implicitly by being declared constexpr,
+ // consteval, etc).
// FIXME: We need a better way to separate C++ standard and clang modules.
bool ImplicitInlineCXX20 = !getLangOpts().CPlusPlusModules ||
NewFD->isConstexpr() || NewFD->isConsteval() ||
More information about the cfe-commits
mailing list