[clang] [clang] When checking for covariant return types, make sure the pointers or references are to *classes* (PR #111856)
Boaz Brickner via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 14 07:19:46 PDT 2024
https://github.com/bricknerb updated https://github.com/llvm/llvm-project/pull/111856
>From 786d31e2657964e578cd1fdf2006b0fb3b19fab6 Mon Sep 17 00:00:00 2001
From: Boaz Brickner <brickner at google.com>
Date: Thu, 10 Oct 2024 15:15:23 +0000
Subject: [PATCH 1/5] [clang] When checking for covariant return types, make
sure the pointers or references are to *classes*.
https://eel.is/c++draft/class.virtual#8.1
This prevents overriding methods with non class return types that have less cv-qualification.
---
clang/lib/Sema/SemaDeclCXX.cpp | 4 ++--
clang/test/SemaCXX/virtual-override.cpp | 10 ++++++++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9cb2ed02a3f764..6195b62b8afa16 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18273,7 +18273,7 @@ bool Sema::CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
}
// The return types aren't either both pointers or references to a class type.
- if (NewClassTy.isNull()) {
+ if (NewClassTy.isNull() || !NewClassTy->isStructureOrClassType()) {
Diag(New->getLocation(),
diag::err_different_return_type_for_overriding_virtual_function)
<< New->getDeclName() << NewTy << OldTy
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
diag::err_covariant_return_incomplete,
New->getDeclName()))
return true;
- }
+ }
// Check if the new class derives from the old class.
if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {
diff --git a/clang/test/SemaCXX/virtual-override.cpp b/clang/test/SemaCXX/virtual-override.cpp
index 72abfc3cf51e1f..6008b8ed063f20 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -289,3 +289,13 @@ namespace PR8168 {
static void foo() {} // expected-error{{'static' member function 'foo' overrides a virtual function}}
};
}
+
+namespace T13 {
+ struct A {
+ virtual const int *f() const; // expected-note{{overridden virtual function is here}}
+ };
+
+ struct B : A {
+ int *f() const override; // expected-error{{virtual function 'f' has a different return type ('int *') than the function it overrides (which has return type 'const int *')}}
+ };
+}
>From 027a985f2ca2bbe007db751af4fdbb5d8f12959d Mon Sep 17 00:00:00 2001
From: Boaz Brickner <brickner at google.com>
Date: Fri, 11 Oct 2024 05:29:05 +0000
Subject: [PATCH 2/5] Fix indentation.
---
clang/lib/Sema/SemaDeclCXX.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 6195b62b8afa16..75d010dc4e04d8 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18296,7 +18296,7 @@ bool Sema::CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
diag::err_covariant_return_incomplete,
New->getDeclName()))
return true;
- }
+ }
// Check if the new class derives from the old class.
if (!IsDerivedFrom(New->getLocation(), NewClassTy, OldClassTy)) {
>From 0b0452f3b7206fdea595aff684c329fd4563f631 Mon Sep 17 00:00:00 2001
From: Boaz Brickner <brickner at google.com>
Date: Fri, 11 Oct 2024 12:27:00 +0000
Subject: [PATCH 3/5] [clang] Add the breaking change to more correctly check
covariance when return type doesn't point to a class to release notes.
---
clang/docs/ReleaseNotes.rst | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index df165b91252505..55ca61955c5b01 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -99,6 +99,19 @@ C++ Specific Potentially Breaking Changes
// Was error, now evaluates to false.
constexpr bool b = f() == g();
+- Clang will now correctly not consider pointers to non classes for covariance.
+
+ .. code-block:: c++
+
+ struct A {
+ virtual const int *f() const;
+ };
+ struct B : A {
+ // Return type has less cv-qualification but doesn't point to a class.
+ // Error will be generated.
+ int *f() const override;
+ };
+
ABI Changes in This Version
---------------------------
>From d5cf39d317ea2474477382f6dfa3a116c7db67f8 Mon Sep 17 00:00:00 2001
From: Boaz Brickner <brickner at google.com>
Date: Fri, 11 Oct 2024 13:38:39 +0000
Subject: [PATCH 4/5] [clang] Fix code indentation in release notes for fixing
how we handle pointers to non classes when calculating covariance.
---
clang/docs/ReleaseNotes.rst | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 55ca61955c5b01..3ae716859fdcdf 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -103,14 +103,14 @@ C++ Specific Potentially Breaking Changes
.. code-block:: c++
- struct A {
- virtual const int *f() const;
- };
- struct B : A {
- // Return type has less cv-qualification but doesn't point to a class.
- // Error will be generated.
- int *f() const override;
- };
+ struct A {
+ virtual const int *f() const;
+ };
+ struct B : A {
+ // Return type has less cv-qualification but doesn't point to a class.
+ // Error will be generated.
+ int *f() const override;
+ };
ABI Changes in This Version
---------------------------
>From 44d5ebec41cc754ad9063d30bcb916a213e16006 Mon Sep 17 00:00:00 2001
From: Boaz Brickner <brickner at google.com>
Date: Mon, 14 Oct 2024 14:19:30 +0000
Subject: [PATCH 5/5] [clang] Unify the new covariant tests, remove T13 and add
the new tests into T2 and T6. In T2, we check that return types that are
pointers to different non-class types fail. In T6, we check that unlike class
return types, non-class return types do not allow to change qualifiers at all
(add or remove).
---
clang/test/SemaCXX/virtual-override.cpp | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/clang/test/SemaCXX/virtual-override.cpp b/clang/test/SemaCXX/virtual-override.cpp
index 6008b8ed063f20..d37c275d46baeb 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -19,10 +19,12 @@ struct b { };
class A {
virtual a* f(); // expected-note{{overridden virtual function is here}}
+ virtual int *g(); // expected-note{{overridden virtual function is here}}
};
class B : A {
virtual b* f(); // expected-error{{return type of virtual function 'f' is not covariant with the return type of the function it overrides ('b *' is not derived from 'a *')}}
+ virtual char *g(); // expected-error{{virtual function 'g' has a different return type ('char *') than the function it overrides (which has return type 'int *')}}
};
}
@@ -83,11 +85,15 @@ struct a { };
class A {
virtual const a* f();
virtual a* g(); // expected-note{{overridden virtual function is here}}
+ virtual const int* h(); // expected-note{{overridden virtual function is here}}
+ virtual int* i(); // expected-note{{overridden virtual function is here}}
};
class B : A {
virtual a* f();
virtual const a* g(); // expected-error{{return type of virtual function 'g' is not covariant with the return type of the function it overrides (class type 'const a *' is more qualified than class type 'a *'}}
+ virtual int* h(); // expected-error{{virtual function 'h' has a different return type ('int *') than the function it overrides (which has return type 'const int *')}}
+ virtual const int* i(); // expected-error{{virtual function 'i' has a different return type ('const int *') than the function it overrides (which has return type 'int *')}}
};
}
@@ -289,13 +295,3 @@ namespace PR8168 {
static void foo() {} // expected-error{{'static' member function 'foo' overrides a virtual function}}
};
}
-
-namespace T13 {
- struct A {
- virtual const int *f() const; // expected-note{{overridden virtual function is here}}
- };
-
- struct B : A {
- int *f() const override; // expected-error{{virtual function 'f' has a different return type ('int *') than the function it overrides (which has return type 'const int *')}}
- };
-}
More information about the cfe-commits
mailing list