Skip to content

[clang] Mark constructors with invalid initializers as invalid #137773

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tbaederr
Copy link
Contributor

If a member or base initializer of a constructor turns out to be invalid, mark the entire constructor as invalid.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 29, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 29, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

If a member or base initializer of a constructor turns out to be invalid, mark the entire constructor as invalid.


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

5 Files Affected:

  • (modified) clang/lib/Parse/ParseDeclCXX.cpp (+3-1)
  • (modified) clang/test/SemaCXX/class-base-member-init.cpp (+1-1)
  • (modified) clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp (+2-2)
  • (modified) clang/test/SemaCXX/constexpr-subobj-initialization.cpp (+4-4)
  • (modified) clang/test/SemaCXX/constructor-initializer.cpp (+7-3)
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 51fe0663a8d1a..7212ac8daea23 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -4064,8 +4064,10 @@ void Parser::ParseConstructorInitializer(Decl *ConstructorDecl) {
     MemInitResult MemInit = ParseMemInitializer(ConstructorDecl);
     if (!MemInit.isInvalid())
       MemInitializers.push_back(MemInit.get());
-    else
+    else {
+      ConstructorDecl->setInvalidDecl();
       AnyErrors = true;
+    }
 
     if (Tok.is(tok::comma))
       ConsumeToken();
diff --git a/clang/test/SemaCXX/class-base-member-init.cpp b/clang/test/SemaCXX/class-base-member-init.cpp
index a6bb4410a81c8..cf297328c1f83 100644
--- a/clang/test/SemaCXX/class-base-member-init.cpp
+++ b/clang/test/SemaCXX/class-base-member-init.cpp
@@ -86,7 +86,7 @@ namespace test5 {
              decltype(Base(1))(2), // expected-error {{multiple initializations given for base 'decltype(Base(1))' (aka 'test5::Base')}}
              decltype(int())() { // expected-error {{constructor initializer 'decltype(int())' (aka 'int') does not name a class}}
     }
-    A(float) : decltype(A())(3) {
+    A(float) : decltype(A())(3) { // expected-error {{constructor for 'A' creates a delegation cycle}}
     }
   };
 }
diff --git a/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp b/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
index 7a6d7cb353158..474ad587bacc6 100644
--- a/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
+++ b/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
@@ -535,9 +535,9 @@ namespace InvalidBaseClass {
   };
 
   class InBetween : public F{};
-  class E : public InBetween {
+  class E : public InBetween { // expected-note 2{{candidate constructor}}
   public:
     constexpr E() :  F{3} {} // expected-error {{not a direct or virtual base}}
   };
-  static_assert(__builtin_bit_cast(char, E()) == 0); // expected-error {{not an integral constant expression}}
+  static_assert(__builtin_bit_cast(char, E()) == 0); // expected-error {{no matching constructor}}
 }
diff --git a/clang/test/SemaCXX/constexpr-subobj-initialization.cpp b/clang/test/SemaCXX/constexpr-subobj-initialization.cpp
index f0252df1e2ce1..f0abe09d23bf4 100644
--- a/clang/test/SemaCXX/constexpr-subobj-initialization.cpp
+++ b/clang/test/SemaCXX/constexpr-subobj-initialization.cpp
@@ -16,17 +16,17 @@ struct Bar : Foo {
 constexpr Bar bar; // expected-error {{must be initialized by a constant expression}}
 
 struct Base {};
-struct A : Base { // expected-note-re {{constructor of base class '{{.*}}Base' is not called}}
+struct A : Base { // expected-note 2{{candidate constructor}}
   constexpr A() : value() {} // expected-error {{member initializer 'value' does not name a non-static data member or base class}}
 };
 
-constexpr A a; // expected-error {{must be initialized by a constant expression}}
+constexpr A a; // expected-error {{no matching constructor}}
 
-struct B : Base { // expected-note-re {{constructor of base class '{{.*}}Base' is not called}}
+struct B : Base { // expected-note 2{{candidate constructor}}
   constexpr B() : {} // expected-error {{expected class member or base class name}}
 };
 
-constexpr B b; // expected-error {{must be initialized by a constant expression}}
+constexpr B b; // expected-error {{no matching constructor}}
 } // namespace baseclass_uninit
 
 
diff --git a/clang/test/SemaCXX/constructor-initializer.cpp b/clang/test/SemaCXX/constructor-initializer.cpp
index 96be8dda97735..c8b1cfbae65ce 100644
--- a/clang/test/SemaCXX/constructor-initializer.cpp
+++ b/clang/test/SemaCXX/constructor-initializer.cpp
@@ -2,16 +2,20 @@
 // RUN: %clang_cc1 -Wreorder -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -Wreorder -fsyntax-only -verify -std=c++11 %s
 
-class A { 
+class A {
+  // expected-note@-1 {{candidate constructor}}
+#if __cplusplus >= 201103L // C++11 or later
+  // expected-note@-3 {{candidate constructor}}
+#endif
   int m;
 public:
    A() : A::m(17) { } // expected-error {{member initializer 'm' does not name a non-static data member or base class}}
-   A(int);
+   A(int); // expected-note {{candidate constructor}}
 };
 
 class B : public A { 
 public:
-  B() : A(), m(1), n(3.14) { }
+  B() : A(), m(1), n(3.14) { } // expected-error {{no matching constructor for initialization of 'A'}}
 
 private:
   int m;

If a member or base initializer of a constructor turns out to be
invalid, mark the entire constructor as invalid.
@llvmbot llvmbot added the clang:bytecode Issues for the clang bytecode constexpr interpreter label Apr 29, 2025
};

class B : public A {
public:
B() : A(), m(1), n(3.14) { }
B() : A(), m(1), n(3.14) { } // expected-error {{no matching constructor for initialization of 'A'}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem like an improvement. Is there some way we can recover more gracefully for constructors with a valid signature?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof, yeah, this diagnostic is confusing because it sure looks like there's a matching constructor for A.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess not, unless invalid constructors were to take part in lookup later when calling A().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:bytecode Issues for the clang bytecode constexpr interpreter clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants