[clang] [C++20][Modules] Restore inliness of constexpr/consteval functions defined in-class (PR #109470)

via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 20 13:25:46 PDT 2024


https://github.com/tomasz-kaminski-sonarsource created https://github.com/llvm/llvm-project/pull/109470

This correct issue, when the functions declared as `constexpr` are `consteval`, are not considered to be inline (`isInlined()` is false) when defined inside the class attached to the 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/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.

>From ed0bc29d0bdac3a75162ae8aa5c67cf0daad2b5d 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] [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/SonarSource/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



More information about the cfe-commits mailing list