[clang] 1c99907 - [CUDA][HIP] Fix overriding of constexpr virtual function (#121986)

[via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2025-01-09T14:05:27-05:00
New Revision: 1c999072221371c9008cd0aec80e387777378de0

URL: https://github.com/llvm/llvm-project/commit/1c999072221371c9008cd0aec80e387777378de0
DIFF: https://github.com/llvm/llvm-project/commit/1c999072221371c9008cd0aec80e387777378de0.diff

LOG: [CUDA][HIP] Fix overriding of constexpr virtual function (#121986)

In C++20 constexpr virtual function is allowed. In C++17 although
non-pure virtual function is not allowed to be constexpr, pure virtual
function is allowed to be constexpr and is allowed to be overriden by
non-constexpr virtual function in the derived class.

The following code compiles as C++:

```
class A
{
public:
    constexpr virtual int f() = 0;
};

class B : public A
{
public:
    int f() override
    {
        return 42;
    }
};
```

However, it fails to compile as CUDA or HIP code. The reason: A::f() is
implicitly host device function whereas B::f() is a host function. Since
they have different targets, clang does not treat B::f() as an override
of A::f(). Instead, it treats B::f() as a name-hiding non-virtual
function for A::f(), and diagnoses it.

This causes any CUDA/HIP program using C++ standard header file
`<format>` from g++-13 to fail to compile since such usage patten show
up there:

```
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/format:3564:34: error: non-virtual member function marked 'override' hides virtual member function
 3564 |       _M_format_arg(size_t __id) override
      |                                  ^
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/format:3538:30: note: hidden overloaded virtual function 'std::__format::_Scanner<char>::_M_format_arg' declared here
 3538 |       constexpr virtual void _M_format_arg(size_t __id) = 0;
      |                              ^
```

This is a serious issue and there is no workaround.

This patch allows non-constexpr function to override constexpr virtual
function for CUDA and HIP. This should be OK since non-constexpr
function without explicit host or device attribute can only be called in
host functions.

Fixes: SWDEV-507350

Added: 
    clang/test/SemaCUDA/constexpr-virtual-func.cu

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Sema/SemaOverload.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c50260b5488512..5ac886856c539c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1058,6 +1058,7 @@ RISC-V Support
 
 CUDA/HIP Language Changes
 ^^^^^^^^^^^^^^^^^^^^^^^^^
+- Fixed a bug about overriding a constexpr pure-virtual member function with a non-constexpr virtual member function which causes compilation failure when including standard C++ header `format`.
 
 CUDA Support
 ^^^^^^^^^^^^

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 7589701fb81de9..3be9ade80f1d94 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1309,6 +1309,13 @@ Sema::CheckOverload(Scope *S, FunctionDecl *New, const LookupResult &Old,
   return Ovl_Overload;
 }
 
+template <typename AttrT> static bool hasExplicitAttr(const FunctionDecl *D) {
+  assert(D && "function decl should not be null");
+  if (auto *A = D->getAttr<AttrT>())
+    return !A->isImplicit();
+  return false;
+}
+
 static bool IsOverloadOrOverrideImpl(Sema &SemaRef, FunctionDecl *New,
                                      FunctionDecl *Old,
                                      bool UseMemberUsingDeclRules,
@@ -1583,6 +1590,7 @@ static bool IsOverloadOrOverrideImpl(Sema &SemaRef, FunctionDecl *New,
       return true;
   }
 
+  // At this point, it is known that the two functions have the same signature.
   if (SemaRef.getLangOpts().CUDA && ConsiderCudaAttrs) {
     // Don't allow overloading of destructors.  (In theory we could, but it
     // would be a giant change to clang.)
@@ -1595,8 +1603,19 @@ static bool IsOverloadOrOverrideImpl(Sema &SemaRef, FunctionDecl *New,
 
         // Allow overloading of functions with same signature and 
diff erent CUDA
         // target attributes.
-        if (NewTarget != OldTarget)
+        if (NewTarget != OldTarget) {
+          // Special case: non-constexpr function is allowed to override
+          // constexpr virtual function
+          if (OldMethod && NewMethod && OldMethod->isVirtual() &&
+              OldMethod->isConstexpr() && !NewMethod->isConstexpr() &&
+              !hasExplicitAttr<CUDAHostAttr>(Old) &&
+              !hasExplicitAttr<CUDADeviceAttr>(Old) &&
+              !hasExplicitAttr<CUDAHostAttr>(New) &&
+              !hasExplicitAttr<CUDADeviceAttr>(New)) {
+            return false;
+          }
           return true;
+        }
       }
     }
   }

diff  --git a/clang/test/SemaCUDA/constexpr-virtual-func.cu b/clang/test/SemaCUDA/constexpr-virtual-func.cu
new file mode 100644
index 00000000000000..89d909181cd94d
--- /dev/null
+++ b/clang/test/SemaCUDA/constexpr-virtual-func.cu
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fsyntax-only \
+// RUN:            -fcuda-is-device %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only %s
+
+// expected-no-diagnostics
+
+#include "Inputs/cuda.h"
+
+class A
+{
+public:
+    constexpr virtual int f() = 0;
+};
+
+class B : public A
+{
+public:
+    int f() override
+    {
+        return 42;
+    }
+};
+
+int test()
+{
+    B b;
+    return b.f();
+}