https://github.com/memehdi14 updated https://github.com/llvm/llvm-project/pull/228212
>From a9b9cba3561f80ca30a180bdcdde0e3d6354b921 Mon Sep 17 00:00:00 2001 From: MeMehdi14 <[email protected]> Date: Thu, 1 Oct 2026 19:26:45 +0000 Subject: [PATCH 1/7] [Clang][Sema] Diagnose type mismatch on redeclaration after redefinition error When a variable redefinition error occurs, the rejected declaration is marked invalid. In Sema::MergeVarDecl, this invalid declaration was passed to MergeVarDeclTypes, which returns early when either declaration is invalid. As a result, subsequent redeclarations with conflicting types were never diagnosed. Later, constant evaluation of expressions reading the variable would walk the redeclaration chain and locate the original initializer with a mismatched type, causing an assertion failure in IntExprEvaluator::Success. Fix this by merging types against the most recent valid declaration in the redeclaration chain, ensuring type incompatibility errors are emitted and invalid declarations do not bypass semantic type checking. Also verified that related issue #114174 does not crash. Fixes #227733 --- clang/docs/ReleaseNotes.md | 2 ++ clang/lib/Sema/SemaDecl.cpp | 18 +++++++++++++++--- .../Sema/redefinition-then-different-type.c | 10 ++++++++++ 3 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 clang/test/Sema/redefinition-then-different-type.c diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 39aa1bf0a5f433..adc9c5221dac63 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -775,6 +775,8 @@ documentation](CIR/index.md) for more information regarding ClangIR's status and - Fixed an assertion failure when a token followed an `#embed` parameter whose expression evaluated `defined`, such as `#embed <f> limit(defined(x)),`. (#GH212075) +- Fixed an assertion failure in the constant evaluator after a variable redefinition + error followed by a redeclaration with a different type. (#GH227733) - Fixed incorrect handling of C++ import preprocessing token when a digraph character after import. (#GH190693) - Fixed a crash when emitting RTTI for a `dllexport` class, or the fundamental type descriptors for `__cxxabiv1::__fundamental_type_info`, under `-fvisibility=hidden`. (#GH207963) - Fixed an assertion failure when passing a wide string literal to `__builtin_nan`. (#GH212108) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index f3d38a555a524a..f5be81157f58ae 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -4827,15 +4827,27 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) { } // Merge the types. - VarDecl *MostRecent = Old->getMostRecentDecl(); - if (MostRecent != Old) { + VarDecl *TypeOld = Old; + if (TypeOld->isInvalidDecl()) { + for (VarDecl *Prev = Old->getPreviousDecl(); Prev; + Prev = Prev->getPreviousDecl()) { + if (!Prev->isInvalidDecl()) { + TypeOld = Prev; + break; + } + } + } + + VarDecl *MostRecent = TypeOld->getMostRecentDecl(); + if (MostRecent != TypeOld) { MergeVarDeclTypes(New, MostRecent, mergeTypeWithPrevious(*this, New, MostRecent, Previous)); if (New->isInvalidDecl()) return; } - MergeVarDeclTypes(New, Old, mergeTypeWithPrevious(*this, New, Old, Previous)); + MergeVarDeclTypes(New, TypeOld, + mergeTypeWithPrevious(*this, New, TypeOld, Previous)); if (New->isInvalidDecl()) return; diff --git a/clang/test/Sema/redefinition-then-different-type.c b/clang/test/Sema/redefinition-then-different-type.c new file mode 100644 index 00000000000000..f859ad69aa6e89 --- /dev/null +++ b/clang/test/Sema/redefinition-then-different-type.c @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s + +// Previously crashed with an assertion in IntExprEvaluator::Success. +// https://github.com/llvm/llvm-project/issues/227733 + +const char e = 1; // expected-note 2 {{previous definition is here}} +const char e = 1; // expected-error {{redefinition of 'e'}} +const unsigned e; // expected-error {{redefinition of 'e' with a different type: 'const unsigned int' vs 'const char'}} +int x = e; >From dee274274f0936f8b50d17ffacd8453ce418923e Mon Sep 17 00:00:00 2001 From: MeMehdi14 <[email protected]> Date: Mon, 12 Oct 2026 18:03:02 +0000 Subject: [PATCH 2/7] [Clang][Sema] Fix #227733: handle all-invalid decl chains and mismatched initializer types Address reviewer feedback on PR #228212: 1. SemaDecl.cpp (MergeVarDecl): If the entire redeclaration chain is invalid (i.e. no valid previous declaration exists), propagate invalidity to the new declaration via setInvalidDecl(). Previously, the loop fell through and TypeOld remained invalid, causing MergeVarDeclTypes to silently skip type-checking and leave the new declaration in an inconsistent valid state. Also guard MostRecent from being used when it is itself invalid. 2. ExprConstant.cpp (evaluateVarDeclInit): After getAnyInitializer() walks the redeclaration chain and returns an initializer attached to InitDecl (which may differ from VD), verify two things before proceeding: - InitDecl is not invalid. - InitDecl->getType() and VD->getType() are the same or compatible (canonical, unqualified). A mismatch means the initializer was written for a declaration with a different type (e.g. the original const char, while VD is const unsigned), and using it would produce an APSInt with the wrong signedness/width, triggering the assertion in IntExprEvaluator::Success. Return false cleanly in both cases so the evaluator treats the variable as non-constant rather than crashing. 3. ByteCode/Compiler.cpp (visitDeclRef): Bail out with emitError() if the resolved VarDecl is invalid, preventing the new constant interpreter from attempting to evaluate references to declarations that are in an invalid state. 4. Test updates (redefinition-then-different-type.c): - Add -fexperimental-new-constant-interpreter RUN lines (C and C++). - Add coverage for the all-invalid-chain case (two bad-type declarations followed by a valid one). Fixes #227733 --- clang/lib/AST/ByteCode/Compiler.cpp | 2 +- clang/lib/AST/ExprConstant.cpp | 17 ++++++++++++++++- clang/lib/Sema/SemaDecl.cpp | 5 ++++- .../Sema/redefinition-then-different-type.c | 8 ++++++++ 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index 71f761f58aed28..3b2036d360000a 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -8671,7 +8671,7 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) { // Try to lazily visit (or emit dummy pointers for) declarations // we haven't seen yet. const auto *VD = dyn_cast<VarDecl>(D); - if (!VD) + if (!VD || VD->isInvalidDecl()) return this->emitError(E); // For C. diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index f1da1dca7b87b1..78565b325176ad 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -3567,7 +3567,8 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E, // Dig out the initializer, and use the declaration which it's attached to. // FIXME: We should eventually check whether the variable has a reachable // initializing declaration. - const Expr *Init = VD->getAnyInitializer(VD); + const VarDecl *InitDecl = VD; + const Expr *Init = VD->getAnyInitializer(InitDecl); // P2280R4 struck the restriction that variable of reference type should have // a preceding initialization. // Used to be C++20 [expr.const]p5.12: @@ -3583,6 +3584,20 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E, return false; } + if (Init) { + if (InitDecl->isInvalidDecl()) + return false; + + QualType InitType = + Info.Ctx.getCanonicalType(InitDecl->getType()).getUnqualifiedType(); + QualType VDType = + Info.Ctx.getCanonicalType(VD->getType()).getUnqualifiedType(); + if (InitType != VDType && !Info.Ctx.typesAreCompatible(InitType, VDType)) + return false; + + VD = InitDecl; + } + // P2280R4 struck the initialization requirement for variables of reference // type so we can no longer assume we have an Init. // Used to be C++20 [expr.const]p5.12: diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index f5be81157f58ae..1c642f3179f05c 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -4838,8 +4838,11 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) { } } + if (TypeOld->isInvalidDecl()) + return New->setInvalidDecl(); + VarDecl *MostRecent = TypeOld->getMostRecentDecl(); - if (MostRecent != TypeOld) { + if (MostRecent != TypeOld && !MostRecent->isInvalidDecl()) { MergeVarDeclTypes(New, MostRecent, mergeTypeWithPrevious(*this, New, MostRecent, Previous)); if (New->isInvalidDecl()) diff --git a/clang/test/Sema/redefinition-then-different-type.c b/clang/test/Sema/redefinition-then-different-type.c index f859ad69aa6e89..d6d2f99b2d22cb 100644 --- a/clang/test/Sema/redefinition-then-different-type.c +++ b/clang/test/Sema/redefinition-then-different-type.c @@ -1,5 +1,7 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s // RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s +// RUN: %clang_cc1 -fsyntax-only -verify -fexperimental-new-constant-interpreter %s +// RUN: %clang_cc1 -fsyntax-only -verify -fexperimental-new-constant-interpreter -x c++ %s // Previously crashed with an assertion in IntExprEvaluator::Success. // https://github.com/llvm/llvm-project/issues/227733 @@ -8,3 +10,9 @@ const char e = 1; // expected-note 2 {{previous definition is here}} const char e = 1; // expected-error {{redefinition of 'e'}} const unsigned e; // expected-error {{redefinition of 'e' with a different type: 'const unsigned int' vs 'const char'}} int x = e; + +// Test when all previous declarations in the redeclaration chain are invalid. +invalid_type_1 a; // expected-error {{unknown type name 'invalid_type_1'}} +invalid_type_2 a; // expected-error {{unknown type name 'invalid_type_2'}} +int a; +int y = a; >From 483fbc03daa11be9788e47a8c20a6f3b51c1eefd Mon Sep 17 00:00:00 2001 From: MeMehdi14 <[email protected]> Date: Tue, 13 Oct 2026 13:45:33 +0000 Subject: [PATCH 3/7] [Clang][Sema] Address review feedback on #228212: simplify checks and expand tests Address reviewer feedback on PR #228212: 1. SemaDecl.cpp (MergeVarDecl): - Switch to iterating over Old->redecls() to locate the most recent valid declaration in the chain, with a detailed explanatory comment. - When all prior declarations in the chain are invalid, skip type merging instead of prematurely bailing out with setInvalidDecl(), allowing downstream redefinition diagnostics to run normally. 2. ExprConstant.cpp (evaluateVarDeclInit): - Remove redundant type-compatibility check. If InitDecl is invalid, bail out early. If valid, update VD = InitDecl so subsequent evaluation resolves against the declaration owning the initializer. 3. Test updates (redefinition-then-different-type.c): - Pass -std=c23 and use constexpr to force constant evaluation in both C and C++. - Add test coverage for all-invalid declaration chains and valid-invalid-valid-use sequences. --- clang/lib/AST/ExprConstant.cpp | 10 +---- clang/lib/Sema/SemaDecl.cpp | 42 ++++++++++-------- .../Sema/redefinition-then-different-type.c | 44 +++++++++++-------- 3 files changed, 51 insertions(+), 45 deletions(-) diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 78565b325176ad..b7f80b3f1b3233 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -3584,17 +3584,11 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E, return false; } + // If the declaration holding the initializer is itself invalid, bail out + // rather than evaluating a potentially bogus value. if (Init) { if (InitDecl->isInvalidDecl()) return false; - - QualType InitType = - Info.Ctx.getCanonicalType(InitDecl->getType()).getUnqualifiedType(); - QualType VDType = - Info.Ctx.getCanonicalType(VD->getType()).getUnqualifiedType(); - if (InitType != VDType && !Info.Ctx.typesAreCompatible(InitType, VDType)) - return false; - VD = InitDecl; } diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 1c642f3179f05c..04940c7844d92a 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -4827,33 +4827,37 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) { } // Merge the types. - VarDecl *TypeOld = Old; - if (TypeOld->isInvalidDecl()) { - for (VarDecl *Prev = Old->getPreviousDecl(); Prev; - Prev = Prev->getPreviousDecl()) { - if (!Prev->isInvalidDecl()) { - TypeOld = Prev; - break; - } + // + // If Old or any prior declaration in the redeclaration chain is invalid, + // MergeVarDeclTypes will return early without diagnosing a type conflict. + // Walk the chain (using redecls(), which goes newest-to-oldest) to find the + // most recent declaration that is valid so we can still detect type + // mismatches on New. If every prior declaration is invalid, skip the type + // merge entirely to avoid false positives; the redefinition logic below will + // still run and emit the appropriate diagnostic. + VarDecl *TypeOld = nullptr; + for (VarDecl *Prev : Old->redecls()) { + if (!Prev->isInvalidDecl()) { + TypeOld = Prev; + break; } } - if (TypeOld->isInvalidDecl()) - return New->setInvalidDecl(); + if (TypeOld) { + VarDecl *MostRecent = TypeOld->getMostRecentDecl(); + if (MostRecent != TypeOld && !MostRecent->isInvalidDecl()) { + MergeVarDeclTypes(New, MostRecent, + mergeTypeWithPrevious(*this, New, MostRecent, Previous)); + if (New->isInvalidDecl()) + return; + } - VarDecl *MostRecent = TypeOld->getMostRecentDecl(); - if (MostRecent != TypeOld && !MostRecent->isInvalidDecl()) { - MergeVarDeclTypes(New, MostRecent, - mergeTypeWithPrevious(*this, New, MostRecent, Previous)); + MergeVarDeclTypes(New, TypeOld, + mergeTypeWithPrevious(*this, New, TypeOld, Previous)); if (New->isInvalidDecl()) return; } - MergeVarDeclTypes(New, TypeOld, - mergeTypeWithPrevious(*this, New, TypeOld, Previous)); - if (New->isInvalidDecl()) - return; - diag::kind PrevDiag; SourceLocation OldLocation; std::tie(PrevDiag, OldLocation) = diff --git a/clang/test/Sema/redefinition-then-different-type.c b/clang/test/Sema/redefinition-then-different-type.c index d6d2f99b2d22cb..42e1082ecc9608 100644 --- a/clang/test/Sema/redefinition-then-different-type.c +++ b/clang/test/Sema/redefinition-then-different-type.c @@ -1,18 +1,26 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s -// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s -// RUN: %clang_cc1 -fsyntax-only -verify -fexperimental-new-constant-interpreter %s -// RUN: %clang_cc1 -fsyntax-only -verify -fexperimental-new-constant-interpreter -x c++ %s - -// Previously crashed with an assertion in IntExprEvaluator::Success. -// https://github.com/llvm/llvm-project/issues/227733 - -const char e = 1; // expected-note 2 {{previous definition is here}} -const char e = 1; // expected-error {{redefinition of 'e'}} -const unsigned e; // expected-error {{redefinition of 'e' with a different type: 'const unsigned int' vs 'const char'}} -int x = e; - -// Test when all previous declarations in the redeclaration chain are invalid. -invalid_type_1 a; // expected-error {{unknown type name 'invalid_type_1'}} -invalid_type_2 a; // expected-error {{unknown type name 'invalid_type_2'}} -int a; -int y = a; +// RUN: %clang_cc1 -fsyntax-only -verify -std=c23 %s +// RUN: %clang_cc1 -fsyntax-only -verify -fexperimental-new-constant-interpreter -std=c23 %s +// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s +// RUN: %clang_cc1 -fsyntax-only -verify -fexperimental-new-constant-interpreter -x c++ %s + +// Previously crashed with an assertion in IntExprEvaluator::Success. +// https://github.com/llvm/llvm-project/issues/227733 + +constexpr char e = 1; // expected-note 2 {{previous definition is here}} +constexpr char e = 1; // expected-error {{redefinition of 'e'}} +constexpr unsigned e; // expected-error {{redefinition of 'e' with a different type: 'const unsigned int' vs 'const char'}} +int x = e; + +// Test when all previous declarations in the redeclaration chain are invalid: +// nothing should crash and the redefinition chain should still compile cleanly. +invalid_type_1 a; // expected-error {{unknown type name 'invalid_type_1'}} +invalid_type_2 a; // expected-error {{unknown type name 'invalid_type_2'}} +extern int a; +void test_a(void) { int y = a; } + +// Test: valid declaration, followed by an invalid one, followed by a valid one, +// followed by a use. +extern int g; +invalid_type_3 g; // expected-error {{unknown type name 'invalid_type_3'}} +extern int g; +void test_g(void) { int z = g; } >From bbb542ffc9e4f20d858a6ddfe201912d7d5f4628 Mon Sep 17 00:00:00 2001 From: MeMehdi14 <[email protected]> Date: Tue, 13 Oct 2026 18:37:56 +0000 Subject: [PATCH 4/7] [Clang][Sema] Address review feedback on #228212: remove unreachable MostRecent type merge The 'MostRecent != TypeOld && !MostRecent->isInvalidDecl()' branch in MergeVarDecl is dead code: the redecls() loop above already selects the most recent non-invalid declaration, so any declaration newer than TypeOld is necessarily invalid. Removing it also fixes the code_formatter CI failure, which was flagging those lines. --- clang/lib/Sema/SemaDecl.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 04940c7844d92a..13f0e21c0cc83b 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -4844,14 +4844,6 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) { } if (TypeOld) { - VarDecl *MostRecent = TypeOld->getMostRecentDecl(); - if (MostRecent != TypeOld && !MostRecent->isInvalidDecl()) { - MergeVarDeclTypes(New, MostRecent, - mergeTypeWithPrevious(*this, New, MostRecent, Previous)); - if (New->isInvalidDecl()) - return; - } - MergeVarDeclTypes(New, TypeOld, mergeTypeWithPrevious(*this, New, TypeOld, Previous)); if (New->isInvalidDecl()) >From 81ff5694e7c5144c043e827b7e2cb46f2eb74fa5 Mon Sep 17 00:00:00 2001 From: MeMehdi14 <[email protected]> Date: Tue, 13 Oct 2026 18:50:33 +0000 Subject: [PATCH 5/7] [Clang][Sema] Address review feedback on #228212: use llvm::find_if for the first-valid-redecl lookup Replace the manual loop that finds the most recent non-invalid declaration with llvm::find_if over Old->redecls(), matching the existing idiom elsewhere in this file. --- clang/lib/Sema/SemaDecl.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 13f0e21c0cc83b..8bc625a155b530 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -4835,13 +4835,10 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) { // mismatches on New. If every prior declaration is invalid, skip the type // merge entirely to avoid false positives; the redefinition logic below will // still run and emit the appropriate diagnostic. - VarDecl *TypeOld = nullptr; - for (VarDecl *Prev : Old->redecls()) { - if (!Prev->isInvalidDecl()) { - TypeOld = Prev; - break; - } - } + auto ValidIter = llvm::find_if(Old->redecls(), [](const VarDecl *D) { + return !D->isInvalidDecl(); + }); + VarDecl *TypeOld = ValidIter == Old->redecls_end() ? nullptr : *ValidIter; if (TypeOld) { MergeVarDeclTypes(New, TypeOld, >From 97de14369913312e9e9e1c27d27c549162119705 Mon Sep 17 00:00:00 2001 From: MeMehdi14 <[email protected]> Date: Tue, 13 Oct 2026 19:18:24 +0000 Subject: [PATCH 6/7] [Clang][Sema] Address review feedback on #228212: drop TypeOld, use ValidIter directly Apply reviewer suggestion: instead of materializing a VarDecl* from the find_if result and null-checking it, compare the iterator against redecls_end() and dereference at the single use site. --- clang/lib/Sema/SemaDecl.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 8bc625a155b530..ea84fa27faab92 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -4838,11 +4838,10 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) { auto ValidIter = llvm::find_if(Old->redecls(), [](const VarDecl *D) { return !D->isInvalidDecl(); }); - VarDecl *TypeOld = ValidIter == Old->redecls_end() ? nullptr : *ValidIter; - if (TypeOld) { - MergeVarDeclTypes(New, TypeOld, - mergeTypeWithPrevious(*this, New, TypeOld, Previous)); + if (ValidIter != Old->redecls_end()) { + MergeVarDeclTypes(New, *ValidIter, + mergeTypeWithPrevious(*this, New, *ValidIter, Previous)); if (New->isInvalidDecl()) return; } >From 8d0bbdff44a0551c9651bf29bee5638dfda0ea51 Mon Sep 17 00:00:00 2001 From: MeMehdi14 <[email protected]> Date: Wed, 14 Oct 2026 13:15:56 +0000 Subject: [PATCH 7/7] [Clang][Sema] Address review feedback on #228212: fix redecl-chain walk, move tests, simplify ExprConstant guard The llvm::find_if(Old->redecls(), ...) walk selected the wrong declaration to merge against: redecl_iterator starts at the declaration redecls() was called on, so when Old is the first declaration of the chain, the type merge ran against Old instead of the most recent declaration, and type conflicts with intermediate redeclarations went undiagnosed (e.g. the extern int b[] / b[3] / b[4] cases in test/Sema/var-redecl.c, which failed premerge CI). Restore the upstream two-step merge (against getMostRecentDecl(), then against Old, which is also how an incomplete array adopts its bound from a preceding same-scope declaration), walking the most recent declaration back past invalid ones so that a declaration marked invalid by an earlier redefinition error does not silently skip the type check. If the whole chain is invalid, skip the type merge and let the redefinition logic below diagnose New. Also: restore the upstream VD->getAnyInitializer(VD) idiom in evaluateVarDeclInit (the guard is now if (Init && VD->isInvalidDecl()) return false;), and move the regression tests into existing files, deleting test/Sema/redefinition-then-different-type.c. --- clang/lib/AST/ExprConstant.cpp | 13 +++--- clang/lib/Sema/SemaDecl.cpp | 43 ++++++++++++------- .../Sema/redefinition-then-different-type.c | 26 ----------- clang/test/Sema/var-redecl.c | 24 +++++++++++ .../SemaCXX/constant-expression-cxx11.cpp | 10 +++++ 5 files changed, 66 insertions(+), 50 deletions(-) delete mode 100644 clang/test/Sema/redefinition-then-different-type.c diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index b7f80b3f1b3233..dd4b166ed7e342 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -3567,8 +3567,7 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E, // Dig out the initializer, and use the declaration which it's attached to. // FIXME: We should eventually check whether the variable has a reachable // initializing declaration. - const VarDecl *InitDecl = VD; - const Expr *Init = VD->getAnyInitializer(InitDecl); + const Expr *Init = VD->getAnyInitializer(VD); // P2280R4 struck the restriction that variable of reference type should have // a preceding initialization. // Used to be C++20 [expr.const]p5.12: @@ -3585,12 +3584,10 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E, } // If the declaration holding the initializer is itself invalid, bail out - // rather than evaluating a potentially bogus value. - if (Init) { - if (InitDecl->isInvalidDecl()) - return false; - VD = InitDecl; - } + // rather than evaluating a potentially bogus value. getAnyInitializer() + // above redirected VD to the declaration holding the initializer. + if (Init && VD->isInvalidDecl()) + return false; // P2280R4 struck the initialization requirement for variables of reference // type so we can no longer assume we have an Init. diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index ea84fa27faab92..658e3b925b7fe9 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -4828,22 +4828,33 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) { // Merge the types. // - // If Old or any prior declaration in the redeclaration chain is invalid, - // MergeVarDeclTypes will return early without diagnosing a type conflict. - // Walk the chain (using redecls(), which goes newest-to-oldest) to find the - // most recent declaration that is valid so we can still detect type - // mismatches on New. If every prior declaration is invalid, skip the type - // merge entirely to avoid false positives; the redefinition logic below will - // still run and emit the appropriate diagnostic. - auto ValidIter = llvm::find_if(Old->redecls(), [](const VarDecl *D) { - return !D->isInvalidDecl(); - }); - - if (ValidIter != Old->redecls_end()) { - MergeVarDeclTypes(New, *ValidIter, - mergeTypeWithPrevious(*this, New, *ValidIter, Previous)); - if (New->isInvalidDecl()) - return; + // MergeVarDeclTypes returns early if either declaration is invalid, so + // merging against an invalid declaration would silently skip the type + // check on New. Start from the most recent declaration and walk back to + // the most recent one that is valid; if the whole chain is invalid, skip + // the type merge entirely and let the redefinition logic below diagnose + // New. Merging against the most recent declaration catches conflicts + // with the latest type, while the subsequent merge against Old is how an + // incomplete array adopts its bound from a preceding same-scope + // declaration, so both merges are still needed. + VarDecl *MostRecent = Old->getMostRecentDecl(); + while (MostRecent->isInvalidDecl() && MostRecent->getPreviousDecl()) + MostRecent = MostRecent->getPreviousDecl(); + + if (!MostRecent->isInvalidDecl()) { + if (MostRecent != Old) { + MergeVarDeclTypes( + New, MostRecent, + mergeTypeWithPrevious(*this, New, MostRecent, Previous)); + if (New->isInvalidDecl()) + return; + } + if (!Old->isInvalidDecl()) { + MergeVarDeclTypes(New, Old, + mergeTypeWithPrevious(*this, New, Old, Previous)); + if (New->isInvalidDecl()) + return; + } } diag::kind PrevDiag; diff --git a/clang/test/Sema/redefinition-then-different-type.c b/clang/test/Sema/redefinition-then-different-type.c deleted file mode 100644 index 42e1082ecc9608..00000000000000 --- a/clang/test/Sema/redefinition-then-different-type.c +++ /dev/null @@ -1,26 +0,0 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -std=c23 %s -// RUN: %clang_cc1 -fsyntax-only -verify -fexperimental-new-constant-interpreter -std=c23 %s -// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s -// RUN: %clang_cc1 -fsyntax-only -verify -fexperimental-new-constant-interpreter -x c++ %s - -// Previously crashed with an assertion in IntExprEvaluator::Success. -// https://github.com/llvm/llvm-project/issues/227733 - -constexpr char e = 1; // expected-note 2 {{previous definition is here}} -constexpr char e = 1; // expected-error {{redefinition of 'e'}} -constexpr unsigned e; // expected-error {{redefinition of 'e' with a different type: 'const unsigned int' vs 'const char'}} -int x = e; - -// Test when all previous declarations in the redeclaration chain are invalid: -// nothing should crash and the redefinition chain should still compile cleanly. -invalid_type_1 a; // expected-error {{unknown type name 'invalid_type_1'}} -invalid_type_2 a; // expected-error {{unknown type name 'invalid_type_2'}} -extern int a; -void test_a(void) { int y = a; } - -// Test: valid declaration, followed by an invalid one, followed by a valid one, -// followed by a use. -extern int g; -invalid_type_3 g; // expected-error {{unknown type name 'invalid_type_3'}} -extern int g; -void test_g(void) { int z = g; } diff --git a/clang/test/Sema/var-redecl.c b/clang/test/Sema/var-redecl.c index f915403ba92559..81bfa717a70a95 100644 --- a/clang/test/Sema/var-redecl.c +++ b/clang/test/Sema/var-redecl.c @@ -1,4 +1,6 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -verify=expected,c23 -std=c23 %s +// RUN: %clang_cc1 -fsyntax-only -verify=expected,c23 -std=c23 -fexperimental-new-constant-interpreter %s int outer1; // expected-note{{previous definition is here}} extern int outer2; // expected-note{{previous declaration is here}} @@ -68,3 +70,25 @@ void f(int x) { // expected-note {{previous definition is here}} extern int b[]; void g20(void) { extern int b[3]; } // expected-note{{previous declaration is here}} void g21(void) { extern int b[4]; } // expected-error{{redeclaration of 'b' with a different type: 'int[4]' vs 'int[3]'}} + +// GH227733: an invalid redefinition used to suppress the type merge in +// MergeVarDecl, so a later redeclaration with a mismatched type was accepted, +// which crashed the constant evaluator when the variable was used. +#if __STDC_VERSION__ >= 202311L +constexpr char e = 1; // c23-note 2 {{previous definition is here}} +constexpr char e = 1; // c23-error {{redefinition of 'e'}} +constexpr unsigned e; // c23-error {{redefinition of 'e' with a different type: 'const unsigned int' vs 'const char'}} +int x = e; +#endif + +// The same, but where every declaration in the redeclaration chain is invalid. +invalid_type_1 v; // expected-error{{unknown type name 'invalid_type_1'}} +invalid_type_2 v; // expected-error{{unknown type name 'invalid_type_2'}} +extern int v; +void test_v(void) { int y = v; } + +// A valid declaration, an invalid one, then a valid one, then a use. +extern int w; +invalid_type_3 w; // expected-error{{unknown type name 'invalid_type_3'}} +extern int w; +void test_w(void) { int z = w; } diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp index 6e41bb98f18051..6c35b675788a1a 100644 --- a/clang/test/SemaCXX/constant-expression-cxx11.cpp +++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp @@ -2755,3 +2755,13 @@ namespace GH111378 { auto &&L = [&]() { return b; }; } } + +namespace GH227733 { + // An invalid redefinition used to suppress the type merge in MergeVarDecl, + // so this mismatched redeclaration was accepted, crashing the constant + // evaluator when the variable was used. + constexpr char e = 1; // expected-note 2 {{previous definition is here}} + constexpr char e = 1; // expected-error {{redefinition of 'e'}} + constexpr unsigned e = 2; // expected-error {{redefinition of 'e' with a different type: 'const unsigned int' vs 'const char'}} + constexpr int x = e; +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
