[C++20][Modules] Restore inliness of constexpr/consteval functions defined in-class #109464
Conversation
This file contains 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
…fined 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 SonarSource/llvm-project@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.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: None (tomasz-kaminski-sonarsource) ChangesThis correct issue, when the functions declared as export module mod;
struct Clazz {
constexpr void f1() { } // non-inline
constexpr void f2();
friend constexpr void f3() {} // non-inline
};
constexpr void Clazz::f3() {} // inlineThis conflicts with [decl.constexpr] p1: This regression was introduced by https://github.com/SonarSource/llvm-project/commit/97af17c5, where the inline of such a function was accidentally removed. The corresponding wording in [class.friend] and p6 [class.mfct] p1 uses "if" and not "if and only if", thus not implying that these are the only cases where such functions are inline. Full diff: https://github.com/llvm/llvm-project/pull/109464.diff 3 Files Affected:
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index de8805e15bc750..19e91a17e6344a 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 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..04fb5d3d6b03ae 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
\ No newline at end of file
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
|
tomasz-kaminski-sonarsource
deleted the
tk/CPP-5745-inline-fix-upstream
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This correct issue, when the functions declared as
constexprareconsteval,are not considered to be inline (
isInlined()is false) when defined inside class attached to named module:This conflicts with [decl.constexpr] p1:
This regression was introduced by https://github.com/SonarSource/llvm-project/commit/97af17c5, where the inline of such a function was accidentally removed. The corresponding wording in [class.friend] and p6 [class.mfct] p1 uses "if" and not "if and only if", thus not implying that these are the only cases where such functions are inline.