[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Aaron Ballman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaf16a4e131c1: Improve error message for constexpr 
constructors of virtual base classes (authored by NoumanAmir657, committed by 
aaron.ballman).

Changed prior to commit:
  https://reviews.llvm.org/D158540?vs=557571=557573#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp


Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -283,3 +283,12 @@
 return r;
   }
 }
+
+struct Base {
+ constexpr Base() = default;
+};
+struct Derived : virtual Base { // expected-note 3{{virtual base class 
declared here}}
+  constexpr Derived() = default; // expected-error {{default constructor 
cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(const Derived&) = default; // expected-error {{copy 
constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(Derived&&) = default; // expected-error {{move constructor 
cannot be 'constexpr' in a class with virtual base class}}
+};
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7819,10 +7819,17 @@
   : isa(MD))) &&
   MD->isConstexpr() && !Constexpr &&
   MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
-Diag(MD->getBeginLoc(), MD->isConsteval()
-? diag::err_incorrect_defaulted_consteval
-: diag::err_incorrect_defaulted_constexpr)
-<< CSM;
+if (!MD->isConsteval() && RD->getNumVBases()) {
+  Diag(MD->getBeginLoc(), 
diag::err_incorrect_defaulted_constexpr_with_vb)
+  << CSM;
+  for (const auto  : RD->vbases())
+Diag(I.getBeginLoc(), diag::note_constexpr_virtual_base_here);
+} else {
+  Diag(MD->getBeginLoc(), MD->isConsteval()
+  ? diag::err_incorrect_defaulted_consteval
+  : 
diag::err_incorrect_defaulted_constexpr)
+  << CSM;
+}
 // FIXME: Explain why the special member can't be constexpr.
 HadError = true;
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9454,6 +9454,8 @@
 def err_incorrect_defaulted_constexpr : Error<
   "defaulted definition of %sub{select_special_member_kind}0 "
   "is not constexpr">;
+def err_incorrect_defaulted_constexpr_with_vb: Error<
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
virtual base class">;
 def err_incorrect_defaulted_consteval : Error<
   "defaulted declaration of %sub{select_special_member_kind}0 "
   "cannot be consteval because implicit definition is not constexpr">;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -216,6 +216,9 @@
 - The fix-it emitted by ``-Wformat`` for scoped enumerations now take the
   enumeration's underlying type into account instead of suggesting a type just
   based on the format string specifier being used.
+- Clang now displays an improved diagnostic and a note when a defaulted special
+  member is marked ``constexpr`` in a class with a virtual base class
+  (`#64843: `_).
 
 Bug Fixes in This Version
 -


Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -283,3 +283,12 @@
 return r;
   }
 }
+
+struct Base {
+ constexpr Base() = default;
+};
+struct Derived : virtual Base { // expected-note 3{{virtual base class declared here}}
+  constexpr Derived() = default; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(const Derived&) = default; // expected-error {{copy constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(Derived&&) = default; // expected-error {{move constructor cannot be 'constexpr' in a class with virtual base 

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

In D158540#4652792 , @aaron.ballman 
wrote:

> The only thing left are the comments on the release note, but I can fix that 
> up when landing if you need me to commit on your behalf. If so, what name and 
> email address would you like me to use for patch attribution? Otherwise, if 
> you have commit privileges, then you can fix up the release note before 
> landing the changes.

I don't have commit rights. You can use:
name: Nouman Amir
email: noumanamir...@gmail.com


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The only thing left are the comments on the release note, but I can fix that up 
when landing if you need me to commit on your behalf. If so, what name and 
email address would you like me to use for patch attribution? Otherwise, if you 
have commit privileges, then you can fix up the release note before landing the 
changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 updated this revision to Diff 557571.
NoumanAmir657 set the repository for this revision to rG LLVM Github Monorepo.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp


Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -283,3 +283,12 @@
 return r;
   }
 }
+
+struct Base {
+ constexpr Base() = default;
+};
+struct Derived : virtual Base { // expected-note 3{{virtual base class 
declared here}}
+  constexpr Derived() = default; // expected-error {{default constructor 
cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(const Derived&) = default; // expected-error {{copy 
constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(Derived&&) = default; // expected-error {{move constructor 
cannot be 'constexpr' in a class with virtual base class}}
+};
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7819,10 +7819,17 @@
   : isa(MD))) &&
   MD->isConstexpr() && !Constexpr &&
   MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
-Diag(MD->getBeginLoc(), MD->isConsteval()
-? diag::err_incorrect_defaulted_consteval
-: diag::err_incorrect_defaulted_constexpr)
-<< CSM;
+if (!MD->isConsteval() && RD->getNumVBases()) {
+  Diag(MD->getBeginLoc(), 
diag::err_incorrect_defaulted_constexpr_with_vb)
+  << CSM;
+  for (const auto  : RD->vbases())
+Diag(I.getBeginLoc(), diag::note_constexpr_virtual_base_here);
+} else {
+  Diag(MD->getBeginLoc(), MD->isConsteval()
+  ? diag::err_incorrect_defaulted_consteval
+  : 
diag::err_incorrect_defaulted_constexpr)
+  << CSM;
+}
 // FIXME: Explain why the special member can't be constexpr.
 HadError = true;
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9454,6 +9454,8 @@
 def err_incorrect_defaulted_constexpr : Error<
   "defaulted definition of %sub{select_special_member_kind}0 "
   "is not constexpr">;
+def err_incorrect_defaulted_constexpr_with_vb: Error<
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
virtual base class">;
 def err_incorrect_defaulted_consteval : Error<
   "defaulted declaration of %sub{select_special_member_kind}0 "
   "cannot be consteval because implicit definition is not constexpr">;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -216,6 +216,9 @@
 - The fix-it emitted by ``-Wformat`` for scoped enumerations now take the
   enumeration's underlying type into account instead of suggesting a type just
   based on the format string specifier being used.
+- Clang now displays an improved diagnostic and a note when defaulted special 
+  member is constexpr in a class with virtual base class
+  (`#64843: `_).
 
 Bug Fixes in This Version
 -


Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -283,3 +283,12 @@
 return r;
   }
 }
+
+struct Base {
+ constexpr Base() = default;
+};
+struct Derived : virtual Base { // expected-note 3{{virtual base class declared here}}
+  constexpr Derived() = default; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(const Derived&) = default; // expected-error {{copy constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(Derived&&) = default; // expected-error {{move constructor cannot be 'constexpr' in a class with virtual base class}}
+};
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7819,10 +7819,17 @@
   : isa(MD))) &&

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:211-212
   with terminals with dark background colors. This is also more consistent 
with GCC.
+- Clang now displays an improved diagnostic and a note when defaulted special 
+  member is a contexpr in a class with virtual base class
 

aaron.ballman wrote:
> 
It looks like these changes got missed.



Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp:295
+};
\ No newline at end of file


Please add a newline back to the end of the file.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 updated this revision to Diff 557557.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp


Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -283,3 +283,12 @@
 return r;
   }
 }
+
+struct Base {
+ constexpr Base() = default;
+};
+struct Derived : virtual Base { // expected-note 3{{virtual base class 
declared here}}
+  constexpr Derived() = default; // expected-error {{default constructor 
cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(const Derived&) = default; // expected-error {{copy 
constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(Derived&&) = default; // expected-error {{move constructor 
cannot be 'constexpr' in a class with virtual base class}}
+};
\ No newline at end of file
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7814,10 +7814,17 @@
   : isa(MD))) &&
   MD->isConstexpr() && !Constexpr &&
   MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
-Diag(MD->getBeginLoc(), MD->isConsteval()
-? diag::err_incorrect_defaulted_consteval
-: diag::err_incorrect_defaulted_constexpr)
-<< CSM;
+if (!MD->isConsteval() && RD->getNumVBases()) {
+  Diag(MD->getBeginLoc(), 
diag::err_incorrect_defaulted_constexpr_with_vb)
+  << CSM;
+  for (const auto  : RD->vbases())
+Diag(I.getBeginLoc(), diag::note_constexpr_virtual_base_here);
+} else {
+  Diag(MD->getBeginLoc(), MD->isConsteval()
+  ? diag::err_incorrect_defaulted_consteval
+  : 
diag::err_incorrect_defaulted_constexpr)
+  << CSM;
+}
 // FIXME: Explain why the special member can't be constexpr.
 HadError = true;
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9416,6 +9416,8 @@
 def err_incorrect_defaulted_constexpr : Error<
   "defaulted definition of %sub{select_special_member_kind}0 "
   "is not constexpr">;
+def err_incorrect_defaulted_constexpr_with_vb: Error<
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
virtual base class">;
 def err_incorrect_defaulted_consteval : Error<
   "defaulted declaration of %sub{select_special_member_kind}0 "
   "cannot be consteval because implicit definition is not constexpr">;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -208,7 +208,9 @@
   (`#54678: `_).
 - Clang now prints its 'note' diagnostic in cyan instead of black, to be more 
compatible
   with terminals with dark background colors. This is also more consistent 
with GCC.
-
+- Clang now displays an improved diagnostic and a note when defaulted special 
+  member is a constexpr in a class with virtual base class
+  
 Bug Fixes in This Version
 -
 - Fixed an issue where a class template specialization whose declaration is


Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -283,3 +283,12 @@
 return r;
   }
 }
+
+struct Base {
+ constexpr Base() = default;
+};
+struct Derived : virtual Base { // expected-note 3{{virtual base class declared here}}
+  constexpr Derived() = default; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(const Derived&) = default; // expected-error {{copy constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(Derived&&) = default; // expected-error {{move constructor cannot be 'constexpr' in a class with virtual base class}}
+};
\ No newline at end of file
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7814,10 +7814,17 @@
   : isa(MD))) &&
   

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 updated this revision to Diff 557555.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp


Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -283,3 +283,12 @@
 return r;
   }
 }
+
+struct Base {
+ constexpr Base() = default;
+};
+struct Derived : virtual Base { // expected-note 3{{virtual base class 
declared here}}
+  constexpr Derived() = default; // expected-error {{default constructor 
cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(const Derived&) = default; // expected-error {{copy 
constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(Derived&&) = default; // expected-error {{move constructor 
cannot be 'constexpr' in a class with virtual base class}}
+};
\ No newline at end of file
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7814,10 +7814,17 @@
   : isa(MD))) &&
   MD->isConstexpr() && !Constexpr &&
   MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
-Diag(MD->getBeginLoc(), MD->isConsteval()
-? diag::err_incorrect_defaulted_consteval
-: diag::err_incorrect_defaulted_constexpr)
-<< CSM;
+if (!MD->isConsteval() && RD->getNumVBases()) {
+  Diag(MD->getBeginLoc(), diag::err_incorrect_defaulted_constexpr_with_vb)
+  << CSM;
+  for (const auto  : RD->vbases())
+Diag(I.getBeginLoc(), diag::note_constexpr_virtual_base_here);
+} else {
+  Diag(MD->getBeginLoc(), MD->isConsteval()
+  ? diag::err_incorrect_defaulted_consteval
+  : diag::err_incorrect_defaulted_constexpr)
+  << CSM;
+}
 // FIXME: Explain why the special member can't be constexpr.
 HadError = true;
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9416,6 +9416,8 @@
 def err_incorrect_defaulted_constexpr : Error<
   "defaulted definition of %sub{select_special_member_kind}0 "
   "is not constexpr">;
+def err_incorrect_defaulted_constexpr_with_vb: Error<
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
virtual base class">;
 def err_incorrect_defaulted_consteval : Error<
   "defaulted declaration of %sub{select_special_member_kind}0 "
   "cannot be consteval because implicit definition is not constexpr">;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -208,6 +208,8 @@
   (`#54678: `_).
 - Clang now prints its 'note' diagnostic in cyan instead of black, to be more 
compatible
   with terminals with dark background colors. This is also more consistent 
with GCC.
+- Clang now displays an improved diagnostic and a note when defaulted special 
+  member is a contexpr in a class with virtual base class
 
 Bug Fixes in This Version
 -


Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -283,3 +283,12 @@
 return r;
   }
 }
+
+struct Base {
+ constexpr Base() = default;
+};
+struct Derived : virtual Base { // expected-note 3{{virtual base class declared here}}
+  constexpr Derived() = default; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(const Derived&) = default; // expected-error {{copy constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(Derived&&) = default; // expected-error {{move constructor cannot be 'constexpr' in a class with virtual base class}}
+};
\ No newline at end of file
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7814,10 +7814,17 @@
   : isa(MD))) &&
   MD->isConstexpr() && !Constexpr &&
   MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
-Diag(MD->getBeginLoc(), 

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:211-212
   with terminals with dark background colors. This is also more consistent 
with GCC.
+- Clang now displays an improved diagnostic and a note when defaulted special 
+  member is a contexpr in a class with virtual base class
 





Comment at: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp:26-33
+struct Base {
+ constexpr Base() = default;
+};
+struct Derived : virtual Base { // expected-note 3{{virtual base class 
declared here}}
+  constexpr Derived() = default; // expected-error {{default constructor 
cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(const Derived&) = default; // expected-error {{copy 
constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(Derived&&) = default; // expected-error {{move constructor 
cannot be 'constexpr' in a class with virtual base class}}

I think the test coverage should live in 
`clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp` as that's where the 
wording is in the standard for this restriction 
(https://eel.is/c++draft/dcl.constexpr).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

In D158540#4652051 , @aaron.ballman 
wrote:

> In D158540#4651859 , @NoumanAmir657 
> wrote:
>
>> @aaron.Ballman 
>> Do I need to make changes other than this? The virtual base diagnostic 
>> doesn't have a test case in files that would generate it. Should I add the 
>> above example as the test case?
>
> You'll need to add a test case to demonstrate the changes, and the example 
> you had above would work well for the tests. You should also add a release 
> note to `clang/docs/ReleaseNotes.rst` so users know about the improvement. 
> But I think that should basically be all that's left to do here.

Added tests and the release note.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 updated this revision to Diff 557507.
NoumanAmir657 set the repository for this revision to rG LLVM Github Monorepo.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp


Index: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
===
--- clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
+++ clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
@@ -23,6 +23,15 @@
   NoCopyMove ncm;
 };
 
+struct Base {
+ constexpr Base() = default;
+};
+struct Derived : virtual Base { // expected-note 3{{virtual base class 
declared here}}
+  constexpr Derived() = default; // expected-error {{default constructor 
cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(const Derived&) = default; // expected-error {{copy 
constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(Derived&&) = default; // expected-error {{move constructor 
cannot be 'constexpr' in a class with virtual base class}}
+};
+
 // If a function is explicitly defaulted on its first declaration
 //   -- it is implicitly considered to be constexpr if the implicit declaration
 //  would be
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7814,10 +7814,17 @@
   : isa(MD))) &&
   MD->isConstexpr() && !Constexpr &&
   MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
-Diag(MD->getBeginLoc(), MD->isConsteval()
-? diag::err_incorrect_defaulted_consteval
-: diag::err_incorrect_defaulted_constexpr)
-<< CSM;
+if (!MD->isConsteval() && RD->getNumVBases()) {
+  Diag(MD->getBeginLoc(), 
diag::err_incorrect_defaulted_constexpr_with_vb)
+  << CSM;
+  for (const auto  : RD->vbases())
+Diag(I.getBeginLoc(), diag::note_constexpr_virtual_base_here);
+} else {
+  Diag(MD->getBeginLoc(), MD->isConsteval()
+  ? diag::err_incorrect_defaulted_consteval
+  : 
diag::err_incorrect_defaulted_constexpr)
+  << CSM;
+}
 // FIXME: Explain why the special member can't be constexpr.
 HadError = true;
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9416,6 +9416,8 @@
 def err_incorrect_defaulted_constexpr : Error<
   "defaulted definition of %sub{select_special_member_kind}0 "
   "is not constexpr">;
+def err_incorrect_defaulted_constexpr_with_vb: Error<
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
virtual base class">;
 def err_incorrect_defaulted_consteval : Error<
   "defaulted declaration of %sub{select_special_member_kind}0 "
   "cannot be consteval because implicit definition is not constexpr">;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -208,6 +208,8 @@
   (`#54678: `_).
 - Clang now prints its 'note' diagnostic in cyan instead of black, to be more 
compatible
   with terminals with dark background colors. This is also more consistent 
with GCC.
+- Clang now displays an improved diagnostic and a note when defaulted special 
+  member is a contexpr in a class with virtual base class
 
 Bug Fixes in This Version
 -


Index: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
===
--- clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
+++ clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
@@ -23,6 +23,15 @@
   NoCopyMove ncm;
 };
 
+struct Base {
+ constexpr Base() = default;
+};
+struct Derived : virtual Base { // expected-note 3{{virtual base class declared here}}
+  constexpr Derived() = default; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(const Derived&) = default; // expected-error {{copy constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(Derived&&) = default; // expected-error {{move constructor cannot be 'constexpr' in a class with virtual base class}}
+};
+
 // If a function is explicitly defaulted 

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 updated this revision to Diff 557504.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp


Index: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
===
--- clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
+++ clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
@@ -23,6 +23,15 @@
   NoCopyMove ncm;
 };
 
+struct Base {
+ constexpr Base() = default;
+};
+struct Derived : virtual Base { // expected-note 3{{virtual base class 
declared here}}
+  constexpr Derived() = default; // expected-error {{default constructor 
cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(const Derived&) = default; // expected-error {{copy 
constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(Derived&&) = default; // expected-error {{move constructor 
cannot be 'constexpr' in a class with virtual base class}}
+};
+
 // If a function is explicitly defaulted on its first declaration
 //   -- it is implicitly considered to be constexpr if the implicit declaration
 //  would be
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7800,10 +7800,17 @@
   : isa(MD))) &&
   MD->isConstexpr() && !Constexpr &&
   MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
-Diag(MD->getBeginLoc(), MD->isConsteval()
-? diag::err_incorrect_defaulted_consteval
-: diag::err_incorrect_defaulted_constexpr)
-<< CSM;
+if (!MD->isConsteval() && RD->getNumVBases()) {
+  Diag(MD->getBeginLoc(), 
diag::err_incorrect_defaulted_constexpr_with_vb)
+  << CSM;
+  for (const auto  : RD->vbases())
+Diag(I.getBeginLoc(), diag::note_constexpr_virtual_base_here);
+} else {
+  Diag(MD->getBeginLoc(), MD->isConsteval()
+  ? diag::err_incorrect_defaulted_consteval
+  : 
diag::err_incorrect_defaulted_constexpr)
+  << CSM;
+}
 // FIXME: Explain why the special member can't be constexpr.
 HadError = true;
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9395,6 +9395,8 @@
 def err_incorrect_defaulted_constexpr : Error<
   "defaulted definition of %sub{select_special_member_kind}0 "
   "is not constexpr">;
+def err_incorrect_defaulted_constexpr_with_vb: Error<
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
virtual base class">;
 def err_incorrect_defaulted_consteval : Error<
   "defaulted declaration of %sub{select_special_member_kind}0 "
   "cannot be consteval because implicit definition is not constexpr">;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -144,6 +144,8 @@
   tautologies like ``x && !x`` and ``!x || x`` in expressions. This also
   makes ``-Winfinite-recursion`` diagnose more cases.
   (`#56035: `_).
+- Clang now displays an improved diagnostic and a note when defaulted special 
+  member is a contexpr in a class with virtual base class
 
 Bug Fixes in This Version
 -


Index: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
===
--- clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
+++ clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
@@ -23,6 +23,15 @@
   NoCopyMove ncm;
 };
 
+struct Base {
+ constexpr Base() = default;
+};
+struct Derived : virtual Base { // expected-note 3{{virtual base class declared here}}
+  constexpr Derived() = default; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(const Derived&) = default; // expected-error {{copy constructor cannot be 'constexpr' in a class with virtual base class}}
+  constexpr Derived(Derived&&) = default; // expected-error {{move constructor cannot be 'constexpr' in a class with virtual base class}}
+};
+
 // If a function is explicitly defaulted on its first declaration
 //   -- it is implicitly considered to be constexpr if the implicit declaration
 //  would be
Index: clang/lib/Sema/SemaDeclCXX.cpp

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D158540#4651859 , @NoumanAmir657 
wrote:

> @aaron.Ballman 
> Do I need to make changes other than this? The virtual base diagnostic 
> doesn't have a test case in files that would generate it. Should I add the 
> above example as the test case?

You'll need to add a test case to demonstrate the changes, and the example you 
had above would work well for the tests. You should also add a release note to 
`clang/docs/ReleaseNotes.rst` so users know about the improvement. But I think 
that should basically be all that's left to do here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 updated this revision to Diff 557487.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7800,10 +7800,17 @@
   : isa(MD))) &&
   MD->isConstexpr() && !Constexpr &&
   MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
-Diag(MD->getBeginLoc(), MD->isConsteval()
-? diag::err_incorrect_defaulted_consteval
-: diag::err_incorrect_defaulted_constexpr)
-<< CSM;
+if (!MD->isConsteval() && RD->getNumVBases()) {
+  Diag(MD->getBeginLoc(), diag::err_incorrect_defaulted_constexpr_with_vb)
+  << CSM;
+  for (const auto  : RD->vbases())
+Diag(I.getBeginLoc(), diag::note_constexpr_virtual_base_here);
+} else {
+  Diag(MD->getBeginLoc(), MD->isConsteval()
+  ? diag::err_incorrect_defaulted_consteval
+  : diag::err_incorrect_defaulted_constexpr)
+  << CSM;
+}
 // FIXME: Explain why the special member can't be constexpr.
 HadError = true;
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9395,6 +9395,8 @@
 def err_incorrect_defaulted_constexpr : Error<
   "defaulted definition of %sub{select_special_member_kind}0 "
   "is not constexpr">;
+def err_incorrect_defaulted_constexpr_with_vb: Error<
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
virtual base class">;
 def err_incorrect_defaulted_consteval : Error<
   "defaulted declaration of %sub{select_special_member_kind}0 "
   "cannot be consteval because implicit definition is not constexpr">;


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7800,10 +7800,17 @@
   : isa(MD))) &&
   MD->isConstexpr() && !Constexpr &&
   MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
-Diag(MD->getBeginLoc(), MD->isConsteval()
-? diag::err_incorrect_defaulted_consteval
-: diag::err_incorrect_defaulted_constexpr)
-<< CSM;
+if (!MD->isConsteval() && RD->getNumVBases()) {
+  Diag(MD->getBeginLoc(), diag::err_incorrect_defaulted_constexpr_with_vb)
+  << CSM;
+  for (const auto  : RD->vbases())
+Diag(I.getBeginLoc(), diag::note_constexpr_virtual_base_here);
+} else {
+  Diag(MD->getBeginLoc(), MD->isConsteval()
+  ? diag::err_incorrect_defaulted_consteval
+  : diag::err_incorrect_defaulted_constexpr)
+  << CSM;
+}
 // FIXME: Explain why the special member can't be constexpr.
 HadError = true;
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9395,6 +9395,8 @@
 def err_incorrect_defaulted_constexpr : Error<
   "defaulted definition of %sub{select_special_member_kind}0 "
   "is not constexpr">;
+def err_incorrect_defaulted_constexpr_with_vb: Error<
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with virtual base class">;
 def err_incorrect_defaulted_consteval : Error<
   "defaulted declaration of %sub{select_special_member_kind}0 "
   "cannot be consteval because implicit definition is not constexpr">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 updated this revision to Diff 557480.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7800,12 +7800,17 @@
   : isa(MD))) &&
   MD->isConstexpr() && !Constexpr &&
   MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
-Diag(MD->getBeginLoc(), MD->isConsteval()
-? diag::err_incorrect_defaulted_consteval
-: diag::err_incorrect_defaulted_constexpr)
-<< CSM;
-// FIXME: Explain why the special member can't be constexpr.
-HadError = true;
+  if (!MD->isConsteval() && RD->getNumVBases()) {
+Diag(MD->getBeginLoc(), 
diag::err_incorrect_defaulted_constexpr_with_vb)
+<< CSM;
+for (const auto  : RD->vbases())
+  Diag(I.getBeginLoc(), diag::note_constexpr_virtual_base_here);
+  } else {
+Diag(MD->getBeginLoc(), MD->isConsteval()
+? diag::err_incorrect_defaulted_consteval
+: diag::err_incorrect_defaulted_constexpr)
+<< CSM;
+  }
   }
 
   if (First) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9395,6 +9395,8 @@
 def err_incorrect_defaulted_constexpr : Error<
   "defaulted definition of %sub{select_special_member_kind}0 "
   "is not constexpr">;
+def err_incorrect_defaulted_constexpr_with_vb: Error<
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
virtual base class">;
 def err_incorrect_defaulted_consteval : Error<
   "defaulted declaration of %sub{select_special_member_kind}0 "
   "cannot be consteval because implicit definition is not constexpr">;


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7800,12 +7800,17 @@
   : isa(MD))) &&
   MD->isConstexpr() && !Constexpr &&
   MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
-Diag(MD->getBeginLoc(), MD->isConsteval()
-? diag::err_incorrect_defaulted_consteval
-: diag::err_incorrect_defaulted_constexpr)
-<< CSM;
-// FIXME: Explain why the special member can't be constexpr.
-HadError = true;
+  if (!MD->isConsteval() && RD->getNumVBases()) {
+Diag(MD->getBeginLoc(), diag::err_incorrect_defaulted_constexpr_with_vb)
+<< CSM;
+for (const auto  : RD->vbases())
+  Diag(I.getBeginLoc(), diag::note_constexpr_virtual_base_here);
+  } else {
+Diag(MD->getBeginLoc(), MD->isConsteval()
+? diag::err_incorrect_defaulted_consteval
+: diag::err_incorrect_defaulted_constexpr)
+<< CSM;
+  }
   }
 
   if (First) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9395,6 +9395,8 @@
 def err_incorrect_defaulted_constexpr : Error<
   "defaulted definition of %sub{select_special_member_kind}0 "
   "is not constexpr">;
+def err_incorrect_defaulted_constexpr_with_vb: Error<
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with virtual base class">;
 def err_incorrect_defaulted_consteval : Error<
   "defaulted declaration of %sub{select_special_member_kind}0 "
   "cannot be consteval because implicit definition is not constexpr">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 updated this revision to Diff 557477.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7800,10 +7800,18 @@
   : isa(MD))) &&
   MD->isConstexpr() && !Constexpr &&
   MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
-Diag(MD->getBeginLoc(), MD->isConsteval()
+  
+  if (!MD->isConsteval() && RD->getNumVBases()) {
+Diag(MD->getBeginLoc(), 
diag::err_incorrect_defaulted_constexpr_with_vb)
+  << CSM;
+for (const auto  : RD->vbases())
+  Diag(I.getBeginLoc(), diag::note_constexpr_virtual_base_here);
+  } else {
+Diag(MD->getBeginLoc(), MD->isConsteval()
 ? diag::err_incorrect_defaulted_consteval
 : diag::err_incorrect_defaulted_constexpr)
-<< CSM;
+<< CSM;
+  }
 // FIXME: Explain why the special member can't be constexpr.
 HadError = true;
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9395,6 +9395,8 @@
 def err_incorrect_defaulted_constexpr : Error<
   "defaulted definition of %sub{select_special_member_kind}0 "
   "is not constexpr">;
+def err_incorrect_defaulted_constexpr_with_vb: Error<
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
virtual base class">;
 def err_incorrect_defaulted_consteval : Error<
   "defaulted declaration of %sub{select_special_member_kind}0 "
   "cannot be consteval because implicit definition is not constexpr">;


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7800,10 +7800,18 @@
   : isa(MD))) &&
   MD->isConstexpr() && !Constexpr &&
   MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
-Diag(MD->getBeginLoc(), MD->isConsteval()
+  
+  if (!MD->isConsteval() && RD->getNumVBases()) {
+Diag(MD->getBeginLoc(), diag::err_incorrect_defaulted_constexpr_with_vb)
+  << CSM;
+for (const auto  : RD->vbases())
+  Diag(I.getBeginLoc(), diag::note_constexpr_virtual_base_here);
+  } else {
+Diag(MD->getBeginLoc(), MD->isConsteval()
 ? diag::err_incorrect_defaulted_consteval
 : diag::err_incorrect_defaulted_constexpr)
-<< CSM;
+<< CSM;
+  }
 // FIXME: Explain why the special member can't be constexpr.
 HadError = true;
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9395,6 +9395,8 @@
 def err_incorrect_defaulted_constexpr : Error<
   "defaulted definition of %sub{select_special_member_kind}0 "
   "is not constexpr">;
+def err_incorrect_defaulted_constexpr_with_vb: Error<
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with virtual base class">;
 def err_incorrect_defaulted_consteval : Error<
   "defaulted declaration of %sub{select_special_member_kind}0 "
   "cannot be consteval because implicit definition is not constexpr">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

@aaron.Ballman 
Do I need to make changes other than this? The virtual base diagnostic doesn't 
have a test case in files that would generate it. Should I add the above 
example as the test case?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D158540#4643563 , @NoumanAmir657 
wrote:

> **Code:**
>
>   struct Base {
>constexpr Base() = default;
>   };
>   struct Derived : virtual Base {
> constexpr Derived() = default;
>   };
>   
>   struct NoCopyMove {
> constexpr NoCopyMove() {}
> NoCopyMove(const NoCopyMove&);
> NoCopyMove(NoCopyMove&&);
>   };
>   struct S2 {
> constexpr S2() = default;
> constexpr S2(const S2&) = default;
> constexpr S2(S2&&) = default; 
> NoCopyMove ncm;
>   };
>
> **Error Generated**
>
>   ../../llvm-test/x.cpp:5:3: error: default constructor cannot be 'constexpr' 
> in a class with virtual base class
>   5 |   constexpr Derived() = default;
> |   ^
>   ../../llvm-test/x.cpp:4:18: note: virtual base class declared here
>   4 | struct Derived : virtual Base {
> |  ^
>   ../../llvm-test/x.cpp:15:3: error: defaulted definition of copy constructor 
> is not constexpr
>  15 |   constexpr S2(const S2&) = default;
> |   ^
>   ../../llvm-test/x.cpp:16:3: error: defaulted definition of move constructor 
> is not constexpr
>  16 |   constexpr S2(S2&&) = default; 
> |   ^
>   3 errors generated.
>
> @aaron.ballman do you mean like this? This now generates different 
> diagnostics depending on whether a virtual base class is present or not

Yes, this is along the lines of what I was thinking.

In D158540#4648361 , @NoumanAmir657 
wrote:

> @aaron.ballman 
> why the member is not an constexpr can be seen from getNumVBases(). The 
> 'defaultedSpecialMemberIsConstexpr'  returns false whenever getVNumBases is 
> true for this so we can use that to identify when to give which error 
> diagnostic.
> Can you verify whether this would work or is there any problem with this. I 
> haven't uploaded the diff yet

That sounds like a reasonable direction to me!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

@aaron.ballman 
why the member is not an constexpr can be seen from getNumVBases(). The 
'defaultedSpecialMemberIsConstexpr'  returns false whenever getVNumBases is 
true for this so we can use that to identify when to give which error 
diagnostic.
Can you verify whether this would work or is there any problem with this. I 
haven't uploaded the diff yet


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

**Code:**

  struct Base {
   constexpr Base() = default;
  };
  struct Derived : virtual Base {
constexpr Derived() = default;
  };
  
  struct NoCopyMove {
constexpr NoCopyMove() {}
NoCopyMove(const NoCopyMove&);
NoCopyMove(NoCopyMove&&);
  };
  struct S2 {
constexpr S2() = default;
constexpr S2(const S2&) = default;
constexpr S2(S2&&) = default; 
NoCopyMove ncm;
  };

**Error Generated**

  ../../llvm-test/x.cpp:5:3: error: default constructor cannot be 'constexpr' 
in a class with virtual base class
  5 |   constexpr Derived() = default;
|   ^
  ../../llvm-test/x.cpp:4:18: note: virtual base class declared here
  4 | struct Derived : virtual Base {
|  ^
  ../../llvm-test/x.cpp:15:3: error: defaulted definition of copy constructor 
is not constexpr
 15 |   constexpr S2(const S2&) = default;
|   ^
  ../../llvm-test/x.cpp:16:3: error: defaulted definition of move constructor 
is not constexpr
 16 |   constexpr S2(S2&&) = default; 
|   ^
  3 errors generated.

@aaron.ballman do you mean like this? This now generates different diagnostics 
depending on whether a virtual base class is present or not


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D158540#4638605 , @NoumanAmir657 
wrote:

> In D158540#4632457 , @NoumanAmir657 
> wrote:
>
>> @aaron.ballman 
>> This error gets generated on test cases even when a struct/class as no 
>> virtual base class.
>> see this example on here: example 
>>
>> Is this right behaviour? The note for this should not be generated since 
>> this does not have a virtual base class.
>>
>> The example above is from a test case file in clang. The error message is 
>> the one which was supposed to be improved for this patch.
>>
>> Can you clarify this
>
> @aaron.ballman waiting for your clarification on this to make the changes

Sorry about the delayed response -- the issue is that you've modified the 
diagnostic wording but not the logic for when we emit the diagnostic. I think 
you will need to end up modifying `defaultedSpecialMemberIsConstexpr()` to 
return information about *why* the member is not constexpr so that you can 
select between various diagnostic messages. e.g., you're implementing this 
FIXME: 
https://github.com/llvm/llvm-project/blob/c154ba8abeb6f59f85a9bb6fdf7bd79ad0d8c05c/clang/lib/Sema/SemaDeclCXX.cpp#L7817
 This may also require you to add more diagnostics/`%select` uses because there 
are multiple reasons it could have failed. Does that make sense?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

In D158540#4632457 , @NoumanAmir657 
wrote:

> @aaron.ballman 
> This error gets generated on test cases even when a struct/class as no 
> virtual base class.
> see this example on here: example 
>
> Is this right behaviour? The note for this should not be generated since this 
> does not have a virtual base class.
>
> The example above is from a test case file in clang. The error message is the 
> one which was supposed to be improved for this patch.
>
> Can you clarify this

@aaron.ballman waiting for your clarification on this to make the changes


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

@aaron.ballman 
This error gets generated on test cases even when a struct/class as no virtual 
base class.
see this example on here: example 

Is this right behaviour? The note for this should not be generated since this 
does not have a virtual base class.

The example above is from a test case file in clang

Can you clarify this


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

In D158540#4632240 , @aaron.ballman 
wrote:

> In D158540#4629042 , @NoumanAmir657 
> wrote:
>
>> Do you want the note to be like this:
>>
>>   ../llvm-test/x.cpp:5:1: note: virtual base class declared here
>>   5 | struct Derived : virtual Base {
>> | ^
>>   1 error generated.
>
> Close! I was thinking more like:
>
>   ../llvm-test/x.cpp:5:1: note: virtual base class declared here
>   5 | struct Derived : virtual Base {
> |  ^
>   1 error generated.
>
> (so the location points to the base class and not the derived class).

Okay, I will do this then. Thanks for clarifying.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D158540#4629042 , @NoumanAmir657 
wrote:

> Do you want the note to be like this:
>
>   ../llvm-test/x.cpp:5:1: note: virtual base class declared here
>   5 | struct Derived : virtual Base {
> | ^
>   1 error generated.

Close! I was thinking more like:

  ../llvm-test/x.cpp:5:1: note: virtual base class declared here
  5 | struct Derived : virtual Base {
|  ^
  1 error generated.

(so the location points to the base class and not the derived class).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

In D158540#4628958 , @aaron.ballman 
wrote:

> In D158540#4628904 , @NoumanAmir657 
> wrote:
>
>> In D158540#4628860 , 
>> @aaron.ballman wrote:
>>
>>> In D158540#4628629 , 
>>> @NoumanAmir657 wrote:
>>>
 In D158540#4628310 , 
 @aaron.ballman wrote:

> In D158540#4628265 , 
> @NoumanAmir657 wrote:
>
>> No, I don't have code examples that showcase the importance of the note. 
>> As you said the class context would be obvious whenever we run into this 
>> error.
>> The test files also don't show where the note would be helpful.
>
> The crux of https://github.com/llvm/llvm-project/issues/64843 is about 
> the error diagnostic, and that logic hasn't been changed in this patch -- 
> are there other changes that are missing from the patch? The text of the 
> tests shows that the error diagnostic behavior should have changed as 
> well, but I'm not seeing the functional changes to make that happen.

 Can you elaborate further? I did not understand what you mean by 
 functional changes. According to my knowledge, I don't think anything else 
 is missing from the patch.
>>>
>>> The only changes I see in the review are the addition of 
>>> `note_incorrect_defaulted_constexpr` and emitting that note. However, I see 
>>> test cases changing from:
>>>
>>>   constexpr W() __constant = default; // expected-error {{defaulted 
>>> definition of default constructor is not constexpr}}
>>>
>>> to
>>>
>>>   constexpr W() __constant = default; // expected-error {{default 
>>> constructor cannot be 'constexpr' in a class with virtual base classes}} 
>>> expected-note {{see reference to function 'W' in 'W' class}}
>>>
>>> where the error wording is now different, but I don't see any code changes 
>>> to the compiler for the error wording.
>>
>> I changed the error wording in the file DiagnosticSemaKinds.td
>>
>> "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
>> virtual base classes">;
>
> Okay, *now* I am seeing some changes there .. that's really strange, because 
> my previous viewing only showed the *note* changing. I'm very sorry for the 
> confusing noise; it could be that Phab is a bit flaky (it's considerably 
> slower than it usually is).

Do you want the note to be like this:

  ../llvm-test/x.cpp:5:1: note: virtual base class declared here
  5 | struct Derived : virtual Base {
| ^
  1 error generated.




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

In D158540#4628958 , @aaron.ballman 
wrote:

> In D158540#4628904 , @NoumanAmir657 
> wrote:
>
>> In D158540#4628860 , 
>> @aaron.ballman wrote:
>>
>>> In D158540#4628629 , 
>>> @NoumanAmir657 wrote:
>>>
 In D158540#4628310 , 
 @aaron.ballman wrote:

> In D158540#4628265 , 
> @NoumanAmir657 wrote:
>
>> No, I don't have code examples that showcase the importance of the note. 
>> As you said the class context would be obvious whenever we run into this 
>> error.
>> The test files also don't show where the note would be helpful.
>
> The crux of https://github.com/llvm/llvm-project/issues/64843 is about 
> the error diagnostic, and that logic hasn't been changed in this patch -- 
> are there other changes that are missing from the patch? The text of the 
> tests shows that the error diagnostic behavior should have changed as 
> well, but I'm not seeing the functional changes to make that happen.

 Can you elaborate further? I did not understand what you mean by 
 functional changes. According to my knowledge, I don't think anything else 
 is missing from the patch.
>>>
>>> The only changes I see in the review are the addition of 
>>> `note_incorrect_defaulted_constexpr` and emitting that note. However, I see 
>>> test cases changing from:
>>>
>>>   constexpr W() __constant = default; // expected-error {{defaulted 
>>> definition of default constructor is not constexpr}}
>>>
>>> to
>>>
>>>   constexpr W() __constant = default; // expected-error {{default 
>>> constructor cannot be 'constexpr' in a class with virtual base classes}} 
>>> expected-note {{see reference to function 'W' in 'W' class}}
>>>
>>> where the error wording is now different, but I don't see any code changes 
>>> to the compiler for the error wording.
>>
>> I changed the error wording in the file DiagnosticSemaKinds.td
>>
>> "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
>> virtual base classes">;
>
> Okay, *now* I am seeing some changes there .. that's really strange, because 
> my previous viewing only showed the *note* changing. I'm very sorry for the 
> confusing noise; it could be that Phab is a bit flaky (it's considerably 
> slower than it usually is).

Okay, I will change these things.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D158540#4628904 , @NoumanAmir657 
wrote:

> In D158540#4628860 , @aaron.ballman 
> wrote:
>
>> In D158540#4628629 , 
>> @NoumanAmir657 wrote:
>>
>>> In D158540#4628310 , 
>>> @aaron.ballman wrote:
>>>
 In D158540#4628265 , 
 @NoumanAmir657 wrote:

> No, I don't have code examples that showcase the importance of the note. 
> As you said the class context would be obvious whenever we run into this 
> error.
> The test files also don't show where the note would be helpful.

 The crux of https://github.com/llvm/llvm-project/issues/64843 is about the 
 error diagnostic, and that logic hasn't been changed in this patch -- are 
 there other changes that are missing from the patch? The text of the tests 
 shows that the error diagnostic behavior should have changed as well, but 
 I'm not seeing the functional changes to make that happen.
>>>
>>> Can you elaborate further? I did not understand what you mean by functional 
>>> changes. According to my knowledge, I don't think anything else is missing 
>>> from the patch.
>>
>> The only changes I see in the review are the addition of 
>> `note_incorrect_defaulted_constexpr` and emitting that note. However, I see 
>> test cases changing from:
>>
>>   constexpr W() __constant = default; // expected-error {{defaulted 
>> definition of default constructor is not constexpr}}
>>
>> to
>>
>>   constexpr W() __constant = default; // expected-error {{default 
>> constructor cannot be 'constexpr' in a class with virtual base classes}} 
>> expected-note {{see reference to function 'W' in 'W' class}}
>>
>> where the error wording is now different, but I don't see any code changes 
>> to the compiler for the error wording.
>
> I changed the error wording in the file DiagnosticSemaKinds.td
>
> "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
> virtual base classes">;

Okay, *now* I am seeing some changes there .. that's really strange, because my 
previous viewing only showed the *note* changing. I'm very sorry for the 
confusing noise; it could be that Phab is a bit flaky (it's considerably slower 
than it usually is).




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9396
 def err_incorrect_defaulted_constexpr : Error<
-  "defaulted definition of %sub{select_special_member_kind}0 "
-  "is not constexpr">;
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
virtual base classes">;
+def note_incorrect_defaulted_constexpr : Note<

NoumanAmir657 wrote:
> here is the error wording change
Changes the wording to be singular instead of plural.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9397-9398
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
virtual base classes">;
+def note_incorrect_defaulted_constexpr : Note<
+  "see reference to function %1 in %0 class">;
 def err_incorrect_defaulted_consteval : Error<

Let's drop this note entirely (more on this below).



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:7808-7809
+if (!MD->isConsteval()) {
+  Diag(MD->getBeginLoc(), diag::note_incorrect_defaulted_constexpr)
+  << RD << MD;
+}

aaron.ballman wrote:
> I'm not certain I understand how this note helps users -- the note is always 
> attached to the instance method being defaulted, so it will always appear 
> where the class context is obvious. e.g.,
> ```
> struct S {
>   constexpr S() = default; // Can quickly tell we're in class S
>   constexpr S(const S&);
> };
> 
> constexpr S::S(const S&) = default; // Can still quickly tell we're in class S
> ```
> Do you have some code examples where this note helps clarify in ways I'm not 
> seeing from the test coverage?
Instead of issuing this note, I think we should issue 
`diag::note_constexpr_virtual_base_here` and specify the location of a virtual 
base class. This way, the user sees the diagnostic saying "can't do this 
because of a virtual base class" and the note directs the user back to the 
problem.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9396
 def err_incorrect_defaulted_constexpr : Error<
-  "defaulted definition of %sub{select_special_member_kind}0 "
-  "is not constexpr">;
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
virtual base classes">;
+def note_incorrect_defaulted_constexpr : Note<

here is the error wording change


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

In D158540#4628860 , @aaron.ballman 
wrote:

> In D158540#4628629 , @NoumanAmir657 
> wrote:
>
>> In D158540#4628310 , 
>> @aaron.ballman wrote:
>>
>>> In D158540#4628265 , 
>>> @NoumanAmir657 wrote:
>>>
 No, I don't have code examples that showcase the importance of the note. 
 As you said the class context would be obvious whenever we run into this 
 error.
 The test files also don't show where the note would be helpful.
>>>
>>> The crux of https://github.com/llvm/llvm-project/issues/64843 is about the 
>>> error diagnostic, and that logic hasn't been changed in this patch -- are 
>>> there other changes that are missing from the patch? The text of the tests 
>>> shows that the error diagnostic behavior should have changed as well, but 
>>> I'm not seeing the functional changes to make that happen.
>>
>> Can you elaborate further? I did not understand what you mean by functional 
>> changes. According to my knowledge, I don't think anything else is missing 
>> from the patch.
>
> The only changes I see in the review are the addition of 
> `note_incorrect_defaulted_constexpr` and emitting that note. However, I see 
> test cases changing from:
>
>   constexpr W() __constant = default; // expected-error {{defaulted 
> definition of default constructor is not constexpr}}
>
> to
>
>   constexpr W() __constant = default; // expected-error {{default constructor 
> cannot be 'constexpr' in a class with virtual base classes}} expected-note 
> {{see reference to function 'W' in 'W' class}}
>
> where the error wording is now different, but I don't see any code changes to 
> the compiler for the error wording.

I changed the error wording in the file DiagnosticSemaKinds.td

"%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with 
virtual base classes">;


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D158540#4628629 , @NoumanAmir657 
wrote:

> In D158540#4628310 , @aaron.ballman 
> wrote:
>
>> In D158540#4628265 , 
>> @NoumanAmir657 wrote:
>>
>>> No, I don't have code examples that showcase the importance of the note. As 
>>> you said the class context would be obvious whenever we run into this error.
>>> The test files also don't show where the note would be helpful.
>>
>> The crux of https://github.com/llvm/llvm-project/issues/64843 is about the 
>> error diagnostic, and that logic hasn't been changed in this patch -- are 
>> there other changes that are missing from the patch? The text of the tests 
>> shows that the error diagnostic behavior should have changed as well, but 
>> I'm not seeing the functional changes to make that happen.
>
> Can you elaborate further? I did not understand what you mean by functional 
> changes. According to my knowledge, I don't think anything else is missing 
> from the patch.

The only changes I see in the review are the addition of 
`note_incorrect_defaulted_constexpr` and emitting that note. However, I see 
test cases changing from:

  constexpr W() __constant = default; // expected-error {{defaulted definition 
of default constructor is not constexpr}}

to

  constexpr W() __constant = default; // expected-error {{default constructor 
cannot be 'constexpr' in a class with virtual base classes}} expected-note 
{{see reference to function 'W' in 'W' class}}

where the error wording is now different, but I don't see any code changes to 
the compiler for the error wording.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

In D158540#4628310 , @aaron.ballman 
wrote:

> In D158540#4628265 , @NoumanAmir657 
> wrote:
>
>> No, I don't have code examples that showcase the importance of the note. As 
>> you said the class context would be obvious whenever we run into this error.
>> The test files also don't show where the note would be helpful.
>
> The crux of https://github.com/llvm/llvm-project/issues/64843 is about the 
> error diagnostic, and that logic hasn't been changed in this patch -- are 
> there other changes that are missing from the patch? The text of the tests 
> shows that the error diagnostic behavior should have changed as well, but I'm 
> not seeing the functional changes to make that happen.

Can you elaborate further? I did not understand what you mean by functional 
changes. According to my knowledge, I don't think anything else is missing from 
the patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D158540#4628265 , @NoumanAmir657 
wrote:

> No, I don't have code examples that showcase the importance of the note. As 
> you said the class context would be obvious whenever we run into this error.
> The test files also don't show where the note would be helpful.

The crux of https://github.com/llvm/llvm-project/issues/64843 is about the 
error diagnostic, and that logic hasn't been changed in this patch -- are there 
other changes that are missing from the patch? The text of the tests shows that 
the error diagnostic behavior should have changed as well, but I'm not seeing 
the functional changes to make that happen.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

No, I don't have code examples that showcase the importance of the note. As you 
said the class context would be obvious whenever we run into this error.
The test files also don't show where the note would be helpful.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:7808-7809
+if (!MD->isConsteval()) {
+  Diag(MD->getBeginLoc(), diag::note_incorrect_defaulted_constexpr)
+  << RD << MD;
+}

I'm not certain I understand how this note helps users -- the note is always 
attached to the instance method being defaulted, so it will always appear where 
the class context is obvious. e.g.,
```
struct S {
  constexpr S() = default; // Can quickly tell we're in class S
  constexpr S(const S&);
};

constexpr S::S(const S&) = default; // Can still quickly tell we're in class S
```
Do you have some code examples where this note helps clarify in ways I'm not 
seeing from the test coverage?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

@xgupta the build is successful now. Earlier it failed due to format issues.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 updated this revision to Diff 554629.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
  clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
  clang/test/CXX/drs/dr13xx.cpp
  clang/test/CXX/drs/dr14xx.cpp
  clang/test/CXX/special/class.copy/p13-0x.cpp
  clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp

Index: clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
===
--- clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
+++ clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
@@ -54,5 +54,5 @@
 
 struct W {
   int w;
-  constexpr W() __constant = default; // expected-error {{defaulted definition of default constructor is not constexpr}}
+  constexpr W() __constant = default; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'W' in 'W' class}}
 };
Index: clang/test/CXX/special/class.copy/p13-0x.cpp
===
--- clang/test/CXX/special/class.copy/p13-0x.cpp
+++ clang/test/CXX/special/class.copy/p13-0x.cpp
@@ -125,7 +125,7 @@
 mutable A a;
   };
   struct C {
-constexpr C(const C &) = default; // expected-error {{not constexpr}}
+constexpr C(const C &) = default; // expected-error {{copy constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'C' in 'C' class}}
 A a;
   };
 }
Index: clang/test/CXX/drs/dr14xx.cpp
===
--- clang/test/CXX/drs/dr14xx.cpp
+++ clang/test/CXX/drs/dr14xx.cpp
@@ -129,19 +129,19 @@
 union A { constexpr A() = default; };
 union B { int n; constexpr B() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note@-2 {{see reference to function 'B' in 'B' class}}
 #endif
 union C { int n = 0; constexpr C() = default; };
 struct D { union {}; constexpr D() = default; }; // expected-error {{does not declare anything}}
 struct E { union { int n; }; constexpr E() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note@-2 {{see reference to function 'E' in 'E' class}}
 #endif
 struct F { union { int n = 0; }; constexpr F() = default; };
 
 struct G { union { int n = 0; }; union { int m; }; constexpr G() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note@-2 {{see reference to function 'G' in 'G' class}}
 #endif
 struct H {
   union {
Index: clang/test/CXX/drs/dr13xx.cpp
===
--- clang/test/CXX/drs/dr13xx.cpp
+++ clang/test/CXX/drs/dr13xx.cpp
@@ -328,10 +328,10 @@
 namespace dr1359 { // dr1359: 3.5
 #if __cplusplus >= 201103L
   union A { constexpr A() = default; };
-  union B { constexpr B() = default; int a; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
-  union C { constexpr C() = default; int a, b; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
+  union B { constexpr B() = default; int a; }; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'B' in 'B' class}} expected-note 2{{candidate}}
+  union C { constexpr C() = default; int a, b; }; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'C' in 'C' class}} expected-note 2{{candidate}}
   struct X { constexpr X() = default; union {}; }; // expected-error {{does not declare anything}}
-  struct Y { constexpr Y() = default; union { int a; }; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
+  struct Y { constexpr Y() = default; union { int a; }; }; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'Y' in 'Y' class}} expected-note 2{{candidate}}
 
   constexpr A a = A();
   constexpr B b = B(); // expected-error {{no matching}}
Index: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
===
--- clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
+++ 

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 updated this revision to Diff 554626.
NoumanAmir657 set the repository for this revision to rG LLVM Github Monorepo.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
  clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
  clang/test/CXX/drs/dr13xx.cpp
  clang/test/CXX/drs/dr14xx.cpp
  clang/test/CXX/special/class.copy/p13-0x.cpp
  clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp

Index: clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
===
--- clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
+++ clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
@@ -54,5 +54,5 @@
 
 struct W {
   int w;
-  constexpr W() __constant = default; // expected-error {{defaulted definition of default constructor is not constexpr}}
+  constexpr W() __constant = default; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'W' in 'W' class}}
 };
Index: clang/test/CXX/special/class.copy/p13-0x.cpp
===
--- clang/test/CXX/special/class.copy/p13-0x.cpp
+++ clang/test/CXX/special/class.copy/p13-0x.cpp
@@ -125,7 +125,7 @@
 mutable A a;
   };
   struct C {
-constexpr C(const C &) = default; // expected-error {{not constexpr}}
+constexpr C(const C &) = default; // expected-error {{copy constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'C' in 'C' class}}
 A a;
   };
 }
Index: clang/test/CXX/drs/dr14xx.cpp
===
--- clang/test/CXX/drs/dr14xx.cpp
+++ clang/test/CXX/drs/dr14xx.cpp
@@ -129,19 +129,19 @@
 union A { constexpr A() = default; };
 union B { int n; constexpr B() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note@-2 {{see reference to function 'B' in 'B' class}}
 #endif
 union C { int n = 0; constexpr C() = default; };
 struct D { union {}; constexpr D() = default; }; // expected-error {{does not declare anything}}
 struct E { union { int n; }; constexpr E() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note@-2 {{see reference to function 'E' in 'E' class}}
 #endif
 struct F { union { int n = 0; }; constexpr F() = default; };
 
 struct G { union { int n = 0; }; union { int m; }; constexpr G() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note@-2 {{see reference to function 'G' in 'G' class}}
 #endif
 struct H {
   union {
Index: clang/test/CXX/drs/dr13xx.cpp
===
--- clang/test/CXX/drs/dr13xx.cpp
+++ clang/test/CXX/drs/dr13xx.cpp
@@ -328,10 +328,10 @@
 namespace dr1359 { // dr1359: 3.5
 #if __cplusplus >= 201103L
   union A { constexpr A() = default; };
-  union B { constexpr B() = default; int a; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
-  union C { constexpr C() = default; int a, b; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
+  union B { constexpr B() = default; int a; }; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'B' in 'B' class}} expected-note 2{{candidate}}
+  union C { constexpr C() = default; int a, b; }; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'C' in 'C' class}} expected-note 2{{candidate}}
   struct X { constexpr X() = default; union {}; }; // expected-error {{does not declare anything}}
-  struct Y { constexpr Y() = default; union { int a; }; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
+  struct Y { constexpr Y() = default; union { int a; }; }; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'Y' in 'Y' class}} expected-note 2{{candidate}}
 
   constexpr A a = A();
   constexpr B b = B(); // expected-error {{no matching}}
Index: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Shivam Gupta via Phabricator via cfe-commits]

xgupta accepted this revision.
xgupta added a comment.
This revision is now accepted and ready to land.

LGTM, Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 updated this revision to Diff 554571.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
  clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
  clang/test/CXX/drs/dr13xx.cpp
  clang/test/CXX/drs/dr14xx.cpp
  clang/test/CXX/special/class.copy/p13-0x.cpp
  clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp

Index: clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
===
--- clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
+++ clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
@@ -54,5 +54,5 @@
 
 struct W {
   int w;
-  constexpr W() __constant = default; // expected-error {{defaulted definition of default constructor is not constexpr}}
+  constexpr W() __constant = default; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'W' in 'W' class}}
 };
Index: clang/test/CXX/special/class.copy/p13-0x.cpp
===
--- clang/test/CXX/special/class.copy/p13-0x.cpp
+++ clang/test/CXX/special/class.copy/p13-0x.cpp
@@ -125,7 +125,7 @@
 mutable A a;
   };
   struct C {
-constexpr C(const C &) = default; // expected-error {{not constexpr}}
+constexpr C(const C &) = default; // expected-error {{copy constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'C' in 'C' class}}
 A a;
   };
 }
Index: clang/test/CXX/drs/dr14xx.cpp
===
--- clang/test/CXX/drs/dr14xx.cpp
+++ clang/test/CXX/drs/dr14xx.cpp
@@ -129,19 +129,19 @@
 union A { constexpr A() = default; };
 union B { int n; constexpr B() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note@-2 {{see reference to function 'B' in 'B' class}}
 #endif
 union C { int n = 0; constexpr C() = default; };
 struct D { union {}; constexpr D() = default; }; // expected-error {{does not declare anything}}
 struct E { union { int n; }; constexpr E() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note@-2 {{see reference to function 'E' in 'E' class}}
 #endif
 struct F { union { int n = 0; }; constexpr F() = default; };
 
 struct G { union { int n = 0; }; union { int m; }; constexpr G() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note@-2 {{see reference to function 'G' in 'G' class}}
 #endif
 struct H {
   union {
Index: clang/test/CXX/drs/dr13xx.cpp
===
--- clang/test/CXX/drs/dr13xx.cpp
+++ clang/test/CXX/drs/dr13xx.cpp
@@ -328,10 +328,10 @@
 namespace dr1359 { // dr1359: 3.5
 #if __cplusplus >= 201103L
   union A { constexpr A() = default; };
-  union B { constexpr B() = default; int a; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
-  union C { constexpr C() = default; int a, b; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
+  union B { constexpr B() = default; int a; }; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'B' in 'B' class}} expected-note 2{{candidate}}
+  union C { constexpr C() = default; int a, b; }; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'C' in 'C' class}} expected-note 2{{candidate}}
   struct X { constexpr X() = default; union {}; }; // expected-error {{does not declare anything}}
-  struct Y { constexpr Y() = default; union { int a; }; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
+  struct Y { constexpr Y() = default; union { int a; }; }; // expected-error {{default constructor cannot be 'constexpr' in a class with virtual base classes}} expected-note {{see reference to function 'Y' in 'Y' class}} expected-note 2{{candidate}}
 
   constexpr A a = A();
   constexpr B b = B(); // expected-error {{no matching}}
Index: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
===
--- clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
+++ 

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Shivam Gupta via Phabricator via cfe-commits]

xgupta added a subscriber: rsmith.
xgupta added a comment.

Just address @rsmith, I think after that we are fine to commit this review.

> +  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class or 
> struct with virtual base classes">;

Please don't say "class or struct" here. Either just say "class" (a struct is a 
kind of class) or actually figure out which one it is and say that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Richard Smith via cfe-commits]

On Tue, 22 Aug 2023 at 12:43, Nouman Amir via Phabricator via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
> ===
> --- clang/include/clang/Basic/DiagnosticSemaKinds.td
> +++ clang/include/clang/Basic/DiagnosticSemaKinds.td
> @@ -9393,8 +9393,7 @@
>"the parameter for an explicitly-defaulted copy assignment operator
> must be an "
>"lvalue reference type">;
>  def err_incorrect_defaulted_constexpr : Error<
> -  "defaulted definition of %sub{select_special_member_kind}0 "
> -  "is not constexpr">;
> +  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class or
> struct with virtual base classes">;
>

Please don't say "class or struct" here. Either just say "class" (a struct
is a kind of class) or actually figure out which one it is and say that.


>  def err_incorrect_defaulted_consteval : Error<
>"defaulted declaration of %sub{select_special_member_kind}0 "
>"cannot be consteval because implicit definition is not constexpr">;
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 updated this revision to Diff 554489.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
  clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
  clang/test/CXX/drs/dr13xx.cpp
  clang/test/CXX/drs/dr14xx.cpp
  clang/test/CXX/special/class.copy/p13-0x.cpp
  clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp

Index: clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
===
--- clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
+++ clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
@@ -54,5 +54,5 @@
 
 struct W {
   int w;
-  constexpr W() __constant = default; // expected-error {{defaulted definition of default constructor is not constexpr}}
+  constexpr W() __constant = default; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note {{see reference to function 'W' in 'W' class or struct}}
 };
Index: clang/test/CXX/special/class.copy/p13-0x.cpp
===
--- clang/test/CXX/special/class.copy/p13-0x.cpp
+++ clang/test/CXX/special/class.copy/p13-0x.cpp
@@ -125,7 +125,7 @@
 mutable A a;
   };
   struct C {
-constexpr C(const C &) = default; // expected-error {{not constexpr}}
+constexpr C(const C &) = default; // expected-error {{copy constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note {{see reference to function 'C' in 'C' class or struct}}
 A a;
   };
 }
Index: clang/test/CXX/drs/dr14xx.cpp
===
--- clang/test/CXX/drs/dr14xx.cpp
+++ clang/test/CXX/drs/dr14xx.cpp
@@ -129,19 +129,19 @@
 union A { constexpr A() = default; };
 union B { int n; constexpr B() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note@-2 {{see reference to function 'B' in 'B' class or struct}}
 #endif
 union C { int n = 0; constexpr C() = default; };
 struct D { union {}; constexpr D() = default; }; // expected-error {{does not declare anything}}
 struct E { union { int n; }; constexpr E() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note@-2 {{see reference to function 'E' in 'E' class or struct}}
 #endif
 struct F { union { int n = 0; }; constexpr F() = default; };
 
 struct G { union { int n = 0; }; union { int m; }; constexpr G() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note@-2 {{see reference to function 'G' in 'G' class or struct}}
 #endif
 struct H {
   union {
Index: clang/test/CXX/drs/dr13xx.cpp
===
--- clang/test/CXX/drs/dr13xx.cpp
+++ clang/test/CXX/drs/dr13xx.cpp
@@ -328,10 +328,10 @@
 namespace dr1359 { // dr1359: 3.5
 #if __cplusplus >= 201103L
   union A { constexpr A() = default; };
-  union B { constexpr B() = default; int a; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
-  union C { constexpr C() = default; int a, b; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
+  union B { constexpr B() = default; int a; }; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note {{see reference to function 'B' in 'B' class or struct}} expected-note 2{{candidate}}
+  union C { constexpr C() = default; int a, b; }; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note {{see reference to function 'C' in 'C' class or struct}} expected-note 2{{candidate}}
   struct X { constexpr X() = default; union {}; }; // expected-error {{does not declare anything}}
-  struct Y { constexpr Y() = default; union { int a; }; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
+  struct Y { constexpr Y() = default; union { int a; }; }; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note {{see reference to function 'Y' in 'Y' class or struct}} expected-note 2{{candidate}}
 
   constexpr A a = A();
   constexpr B b = B(); // expected-error {{no matching}}
Index: 

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 updated this revision to Diff 554485.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
  clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
  clang/test/CXX/drs/dr13xx.cpp
  clang/test/CXX/drs/dr14xx.cpp
  clang/test/CXX/special/class.copy/p13-0x.cpp
  clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp

Index: clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
===
--- clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
+++ clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
@@ -54,5 +54,5 @@
 
 struct W {
   int w;
-  constexpr W() __constant = default; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}}
+  constexpr W() __constant = default; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note {{see reference to function 'W' in 'W' class or struct}}
 };
Index: clang/test/CXX/special/class.copy/p13-0x.cpp
===
--- clang/test/CXX/special/class.copy/p13-0x.cpp
+++ clang/test/CXX/special/class.copy/p13-0x.cpp
@@ -125,7 +125,7 @@
 mutable A a;
   };
   struct C {
-constexpr C(const C &) = default; // expected-error {{copy constructor cannot be 'constexpr' in a class or struct with virtual base classes}}
+constexpr C(const C &) = default; // expected-error {{copy constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note {{see reference to function 'C' in 'C' class or struct}}
 A a;
   };
 }
Index: clang/test/CXX/drs/dr14xx.cpp
===
--- clang/test/CXX/drs/dr14xx.cpp
+++ clang/test/CXX/drs/dr14xx.cpp
@@ -129,19 +129,19 @@
 union A { constexpr A() = default; };
 union B { int n; constexpr B() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note@-2 {{see reference to function 'B' in 'B' class or struct}}
 #endif
 union C { int n = 0; constexpr C() = default; };
 struct D { union {}; constexpr D() = default; }; // expected-error {{does not declare anything}}
 struct E { union { int n; }; constexpr E() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note@-2 {{see reference to function 'E' in 'E' class or struct}}
 #endif
 struct F { union { int n = 0; }; constexpr F() = default; };
 
 struct G { union { int n = 0; }; union { int m; }; constexpr G() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note@-2 {{see reference to function 'G' in 'G' class or struct}}
 #endif
 struct H {
   union {
Index: clang/test/CXX/drs/dr13xx.cpp
===
--- clang/test/CXX/drs/dr13xx.cpp
+++ clang/test/CXX/drs/dr13xx.cpp
@@ -328,10 +328,10 @@
 namespace dr1359 { // dr1359: 3.5
 #if __cplusplus >= 201103L
   union A { constexpr A() = default; };
-  union B { constexpr B() = default; int a; }; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note 2{{candidate}}
-  union C { constexpr C() = default; int a, b; }; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note 2{{candidate}}
+  union B { constexpr B() = default; int a; }; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note {{see reference to function 'B' in 'B' class or struct}} expected-note 2{{candidate}}
+  union C { constexpr C() = default; int a, b; }; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note {{see reference to function 'C' in 'C' class or struct}} expected-note 2{{candidate}}
   struct X { constexpr X() = default; union {}; }; // expected-error {{does not declare anything}}
-  struct Y { constexpr Y() = default; union { int a; }; 

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Shivam Gupta via Phabricator via cfe-commits]

xgupta added a comment.

In D158540#4620286 , @NoumanAmir657 
wrote:

> @xgupta  It passed the test cases now

Thanks, I think we also want a note similar to MSVC diagnostic:

(6): note: see reference to function 'Derived::Derived(void)'


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

@xgupta  It passed the test cases now


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 updated this revision to Diff 553810.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
  clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
  clang/test/CXX/drs/dr13xx.cpp
  clang/test/CXX/drs/dr14xx.cpp
  clang/test/CXX/special/class.copy/p13-0x.cpp
  clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp

Index: clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
===
--- clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
+++ clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
@@ -54,5 +54,5 @@
 
 struct W {
   int w;
-  constexpr W() __constant = default; // expected-error {{defaulted definition of default constructor is not constexpr}}
+  constexpr W() __constant = default; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}}
 };
Index: clang/test/CXX/special/class.copy/p13-0x.cpp
===
--- clang/test/CXX/special/class.copy/p13-0x.cpp
+++ clang/test/CXX/special/class.copy/p13-0x.cpp
@@ -125,7 +125,7 @@
 mutable A a;
   };
   struct C {
-constexpr C(const C &) = default; // expected-error {{not constexpr}}
+constexpr C(const C &) = default; // expected-error {{copy constructor cannot be 'constexpr' in a class or struct with virtual base classes}}
 A a;
   };
 }
Index: clang/test/CXX/drs/dr14xx.cpp
===
--- clang/test/CXX/drs/dr14xx.cpp
+++ clang/test/CXX/drs/dr14xx.cpp
@@ -129,19 +129,19 @@
 union A { constexpr A() = default; };
 union B { int n; constexpr B() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}}
 #endif
 union C { int n = 0; constexpr C() = default; };
 struct D { union {}; constexpr D() = default; }; // expected-error {{does not declare anything}}
 struct E { union { int n; }; constexpr E() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}}
 #endif
 struct F { union { int n = 0; }; constexpr F() = default; };
 
 struct G { union { int n = 0; }; union { int m; }; constexpr G() = default; };
 #if __cplusplus <= 201703L
-// expected-error@-2 {{not constexpr}}
+// expected-error@-2 {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}}
 #endif
 struct H {
   union {
Index: clang/test/CXX/drs/dr13xx.cpp
===
--- clang/test/CXX/drs/dr13xx.cpp
+++ clang/test/CXX/drs/dr13xx.cpp
@@ -328,10 +328,10 @@
 namespace dr1359 { // dr1359: 3.5
 #if __cplusplus >= 201103L
   union A { constexpr A() = default; };
-  union B { constexpr B() = default; int a; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
-  union C { constexpr C() = default; int a, b; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
+  union B { constexpr B() = default; int a; }; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note 2{{candidate}}
+  union C { constexpr C() = default; int a, b; }; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note 2{{candidate}}
   struct X { constexpr X() = default; union {}; }; // expected-error {{does not declare anything}}
-  struct Y { constexpr Y() = default; union { int a; }; }; // expected-error {{not constexpr}} expected-note 2{{candidate}}
+  struct Y { constexpr Y() = default; union { int a; }; }; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with virtual base classes}} expected-note 2{{candidate}}
 
   constexpr A a = A();
   constexpr B b = B(); // expected-error {{no matching}}
Index: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
===
--- clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
+++ clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
@@ -3,7 +3,7 @@
 // An explicitly-defaulted function may be declared constexpr only if it would
 // have been implicitly declared as constexpr.
 struct S1 {
-  constexpr S1() = default; // expected-error {{defaulted definition of default constructor is not constexpr}}
+  constexpr S1() = default; // expected-error {{default constructor cannot be 'constexpr' in a class or struct with 

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

I figured it out and changed the test files.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

How am I supposed to update tests? I am new to this. I get the files on which 
the tests fail but how do I update them


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Shivam Gupta via Phabricator via cfe-commits]

xgupta added reviewers: shafik, aaron.ballman.
xgupta added a comment.

Test case updates are missing due to which 7 test cases are failing on 
pre-merge check - 
https://buildkite.com/llvm-project/premerge-checks/builds/172995#018a2776-1461-4f98-b12d-bd0521352d50/6-14972.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 created this revision.
NoumanAmir657 added a reviewer: CornedBee.
Herald added a project: All.
NoumanAmir657 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The changes are for better diagnostic/error-messages. The error message of 
Clang, MSVC, and GCC were compared and MSVC gives more detailed error message 
so that is used now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158540

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td


Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9393,8 +9393,7 @@
   "the parameter for an explicitly-defaulted copy assignment operator must be 
an "
   "lvalue reference type">;
 def err_incorrect_defaulted_constexpr : Error<
-  "defaulted definition of %sub{select_special_member_kind}0 "
-  "is not constexpr">;
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class or 
struct with virtual base classes">;
 def err_incorrect_defaulted_consteval : Error<
   "defaulted declaration of %sub{select_special_member_kind}0 "
   "cannot be consteval because implicit definition is not constexpr">;


Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9393,8 +9393,7 @@
   "the parameter for an explicitly-defaulted copy assignment operator must be an "
   "lvalue reference type">;
 def err_incorrect_defaulted_constexpr : Error<
-  "defaulted definition of %sub{select_special_member_kind}0 "
-  "is not constexpr">;
+  "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class or struct with virtual base classes">;
 def err_incorrect_defaulted_consteval : Error<
   "defaulted declaration of %sub{select_special_member_kind}0 "
   "cannot be consteval because implicit definition is not constexpr">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits