[clang] Add covariance tests that make sure we return an error when return value is different in pointer / lvalue ref / rvalue ref

[bricknerb]

Per https://cplusplus.github.io/CWG/issues/960.html.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Boaz Brickner (bricknerb)

Changes

Per https://cplusplus.github.io/CWG/issues/960.html.

---

Full diff: https://github.com/llvm/llvm-project/pull/112853.diff

1 Files Affected:

• (modified) clang/test/SemaCXX/virtual-override.cpp (+24-4)

diff --git a/clang/test/SemaCXX/virtual-override.cpp b/clang/test/SemaCXX/virtual-override.cpp
index ce6dd35e0b56fa..fc2a36937d4b5d 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -83,7 +83,7 @@ namespace T6 {
 struct a { };
 
 class A {
-  // Classes.
+  // Check cv-qualification.
   virtual const a* const_vs_unqualified_class();
   virtual a* unqualified_vs_const_class(); // expected-note{{overridden virtual function is here}}
 
@@ -102,13 +102,23 @@ class A {
   virtual const volatile a* const_volatile_vs_unualified_class();
   virtual a* unqualified_vs_const_volatile_class(); // expected-note{{overridden virtual function is here}}
 
-  // Non Classes.
+  // Check lvalue ref vs rvalue ref vs pointer.
+  virtual a& rvalue_ref();
+  virtual a&& lvalue_ref();
+  virtual a& rvalue_vs_lvalue_ref(); // expected-note{{overridden virtual function is here}}
+  virtual a&& lvalue_vs_rvalue_ref(); // expected-note{{overridden virtual function is here}}
+  virtual a& rvalue_ref_vs_pointer(); // expected-note{{overridden virtual function is here}}
+  virtual a* pointer_vs_rvalue_ref(); // expected-note{{overridden virtual function is here}}
+  virtual a&& lvalue_ref_vs_pointer(); // expected-note{{overridden virtual function is here}}
+  virtual a* pointer_vs_lvalue_ref(); // expected-note{{overridden virtual function is here}}
+
+  // Check non class.
   virtual const int* const_vs_unqualified_non_class(); // expected-note{{overridden virtual function is here}}
   virtual int* unqualified_vs_const_non_class(); // expected-note{{overridden virtual function is here}}
 };
 
 class B : A {
-  // Classes.
+  // Check cv-qualification.
   a* const_vs_unqualified_class() override;
   const a* unqualified_vs_const_class() override; // expected-error{{return type of virtual function 'unqualified_vs_const_class' is not covariant with the return type of the function it overrides (class type 'const a *' does not have the same cv-qualification as or less cv-qualification than class type 'a *')}}
 
@@ -127,7 +137,17 @@ class B : A {
   a* const_volatile_vs_unualified_class() override;
   const volatile a* unqualified_vs_const_volatile_class() override; // expected-error{{return type of virtual function 'unqualified_vs_const_volatile_class' is not covariant with the return type of the function it overrides (class type 'const volatile a *' does not have the same cv-qualification as or less cv-qualification than class type 'a *')}}
 
-  // Non Classes.
+  // Check lvalue ref vs rvalue ref vs pointer.
+  a& rvalue_ref() override;
+  a&& lvalue_ref() override;
+  a&& rvalue_vs_lvalue_ref() override; // expected-error{{virtual function 'rvalue_vs_lvalue_ref' has a different return type ('a &&') than the function it overrides (which has return type 'a &')}}
+  a& lvalue_vs_rvalue_ref() override; // expected-error{{virtual function 'lvalue_vs_rvalue_ref' has a different return type ('a &') than the function it overrides (which has return type 'a &&')}}
+  a* rvalue_ref_vs_pointer() override; // expected-error{{virtual function 'rvalue_ref_vs_pointer' has a different return type ('a *') than the function it overrides (which has return type 'a &')}}
+  a& pointer_vs_rvalue_ref() override; // expected-error{{virtual function 'pointer_vs_rvalue_ref' has a different return type ('a &') than the function it overrides (which has return type 'a *')}}
+  a* lvalue_ref_vs_pointer() override; // expected-error{{virtual function 'lvalue_ref_vs_pointer' has a different return type ('a *') than the function it overrides (which has return type 'a &&')}}
+  a&& pointer_vs_lvalue_ref() override; // expected-error{{virtual function 'pointer_vs_lvalue_ref' has a different return type ('a &&') than the function it overrides (which has return type 'a *')}}
+
+  // Check non class.
   int* const_vs_unqualified_non_class() override; // expected-error{{virtual function 'const_vs_unqualified_non_class' has a different return type ('int *') than the function it overrides (which has return type 'const int *')}}
   const int* unqualified_vs_const_non_class() override; // expected-error{{virtual function 'unqualified_vs_const_non_class' has a different return type ('const int *') than the function it overrides (which has return type 'int *')}}
 };

[bricknerb]

From what I can tell, buildkite failures seem unrelated.

[Endilll]

Can you merge main into your branch? I think that should fix the CI failures.

[bricknerb]

Can you merge main into your branch? I think that should fix the CI failures.

Done.

[Endilll]

You should write a test in clang/test/CXX/drs/cwg0xx.cpp, possibly by moving the test you've written there. While doing that, you should follow the example of other tests in that file, which includes the way expected directives are written, and the presence of // cwg960: NN comment that describes what our support for CWG960 is. Then you need to run clang/www/make_cxx_dr_status script which generated our "C++ DR Status" page in the documentation: https://clang.llvm.org/cxx_dr_status.html

[bricknerb]

You should write a test in clang/test/CXX/drs/cwg0xx.cpp, possibly by moving the test you've written there. While doing that, you should follow the example of other tests in that file, which includes the way expected directives are written, and the presence of // cwg960: NN comment that describes what our support for CWG960 is. Then you need to run clang/www/make_cxx_dr_status script which generated our "C++ DR Status" page in the documentation: https://clang.llvm.org/cxx_dr_status.html

Done.
clang/www/make_cxx_dr_status seems to generate a lot of unrelated changes, so perhaps a separate pull request would make sense if we want to update cxx_dr_status.html.

[Endilll]

clang/www/make_cxx_dr_status seems to generate a lot of unrelated changes, so perhaps a separate pull request would make sense if we want to update cxx_dr_status.html.

Ignore other changes; just include the one that updates CWG960 status.
I'll update the rest as an NFC change.

[Endilll]

I left suggestions for one of the errors you're expecting. You should apply those to the rest of expected directives.

[bricknerb]

clang/www/make_cxx_dr_status seems to generate a lot of unrelated changes, so perhaps a separate pull request would make sense if we want to update cxx_dr_status.html.

Ignore other changes; just include the one that updates CWG960 status. I'll update the rest as an NFC change.

Done.

[Endilll]

Thank you! This is good to go now.