https://github.com/guillem-bartrina-sonarsource updated https://github.com/llvm/llvm-project/pull/214008
>From 315175d6c1f863b90e6983f238f71c32e8a67432 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Wed, 29 Jul 2026 15:22:39 +0200 Subject: [PATCH 01/15] [ASTImporter] Invalidate ImportedTypes cache on Decl import failure A TagDecl's type can get cached as successfully imported in ASTImporter::ImportedTypes before the Decl's own import fails, if a member referencing the type (e.g. an implicit copy constructor) is imported first. That stale entry was never invalidated, so later references to the same type -- directly, or via structural-equivalence comparisons on lambda closures -- could silently resolve to a half-built Decl and crash instead of failing cleanly. Add a unit test and a CTU regression test reproducing the crash. --- clang/lib/AST/ASTImporter.cpp | 6 + .../regression/lambda-import-corruption.cpp | 114 ++++++++++++++++++ clang/unittests/AST/ASTImporterTest.cpp | 36 ++++++ 3 files changed, 156 insertions(+) create mode 100644 clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 3ad71a223903c..0b85636a06598 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -10038,6 +10038,12 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { auto *ToD = CreatedToD; ImportedDecls.erase(Pos); + // Also scrub the imported type mapping, if applicable. Import(Type*) can + // cache a type mapping to a declaration that ultimately fails. + if (const auto *FromTD = dyn_cast<TagDecl>(FromD)) + if (const Type *FromTy = getFromContext().getCanonicalTagType(FromTD).getTypePtr()) + ImportedTypes.erase(FromTy); + // ImportedDecls and ImportedFromDecls are not symmetric. It may happen // (e.g. with namespaces) that several decls from the 'from' context are // mapped to the same decl in the 'to' context. If we removed entries diff --git a/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp b/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp new file mode 100644 index 0000000000000..0ecc0a6c03662 --- /dev/null +++ b/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp @@ -0,0 +1,114 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t + +// Pathological case: a lambda's closure (the anonymous class that +// implements its operator()) is created as a Decl before its members are +// imported. If a member unrelated to the eventual failure -- e.g. the +// implicit copy constructor, whose parameter type is `const ClosureType&` +// -- imports successfully first, that success permanently caches the +// closure's type as "imported" in ASTImporter::ImportedTypes, before the +// member that actually fails (here, operator(), due to an unsupported +// trailing requires-clause) is even reached. That cache is never +// invalidated when the closure's own import later fails, so anything that +// subsequently needs the same type (the DeclRefExpr inside +// `decltype(func(...))` on `rudolf`, below) silently gets the half-built +// closure back instead of a clean failure, producing an inconsistent node +// that crashes downstream. + + +// RUN: %clang_cc1 -std=c++20 -emit-pch -o %t/api.cpp.ast %t/api.cpp +// RUN: %clang_cc1 -std=c++20 -emit-pch -o %t/isolate.cpp.ast %t/isolate.cpp + +// RUN: %clang_extdef_map %t/api.cpp -- -std=c++20 > %t/externalDefMap.tmp.txt +// RUN: %clang_extdef_map %t/isolate.cpp -- -std=c++20 >> %t/externalDefMap.tmp.txt +// On windows, absolute paths generated by extdef_map are not recognized, +// so CSA prepends the workdir path to them. Force relative paths to work +// around this issue. +// RUN: sed -e 's| .*api\.cpp| api.cpp.ast|' -e 's| .*isolate\.cpp| isolate.cpp.ast|' \ +// RUN: %t/externalDefMap.tmp.txt > %t/externalDefMap.txt + +// RUN: %clang_analyze_cc1 -std=c++20 \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config experimental-enable-naive-ctu-analysis=true \ +// RUN: -analyzer-config ctu-dir=%t \ +// RUN: -verify %t/main.cpp + +//--- main.cpp + +namespace ns { + +inline constexpr auto func = []<class T>(const T p) {}; + +} + +void import_api(int v); +void trigger_api(); +void trigger_isolate(); + +void entrypoint() { + import_api(0); // [email protected]:20 {{Division by zero}} +} + +void trigger1() { + trigger_api(); +} + +void trigger2() { + trigger_isolate(); +} + +//--- api.cpp + +template <class> int declval(); + +namespace ns { +int import_ns; + +// This closure fails to import: its call operator's trailing +// requires-clause has no importer support. +inline constexpr auto func = []<class T>(const T p) requires requires { 0; } {}; + +// The DeclRefExpr for `func` in this decltype independently re-resolves +// the closure's type after `func` itself was merged away above. +template <class K> decltype(func(declval<K>())) rudolf(int v); + +} // namespace ns + +void import_isolate(int v); + +void import_api(int v) { + (void)ns::import_ns; + import_isolate(v); +} + +void trigger_api() { + ns::rudolf<void>(0); // fails to import +} + +//--- isolate.cpp + +template <class> int declval(); + +namespace ns { +int import_ns; + +constexpr auto func = []<class T>(const T p) requires requires { 0; } {}; + +// Structural equivalence of the return type accesses the closure's +// definition through its type -- an access that assumes the closure is +// intact. +template <class K> decltype(func(declval<K>())) rudolf(int v) { // no-crash + (void)(42 / v); +} + +} // namespace ns + +void import_isolate(int v) { + (void)ns::import_ns; + (void)(42 / v); // raises "Division by zero" +} + +void trigger_isolate() { + ns::rudolf<void>(0); // fails to import +} diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 503f5da8af90f..038b86edb40fd 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -6608,6 +6608,42 @@ TEST_P(ErrorHandlingTest, ErrorIsPropagatedFromMemberToClass) { EXPECT_FALSE(ImportedOK); } +// A member whose signature refers back to the enclosing class (e.g. a +// copy constructor's `const Self&` parameter) can succeed and cache the +// class's *type* before a later, failing member causes the class's own +// Decl import to fail as a whole. Check that this doesn't leave a stale, +// "successfully imported" entry for the class's type behind: any later, +// independent request to import that type must also fail, not silently +// hand back the half-built class. +TEST_P(ErrorHandlingTest, ImportedTypeCacheIsInvalidatedOnFailure) { + TranslationUnitDecl *FromTU = getTuDecl(std::string(R"( + class X { + void ok(const X &) {} // Succeeds; imports X's own type + // as a side effect, before X's + // own import is known to fail. + void bad() { )") + ErroneousStmt + R"( } // Fails to import. + }; + )", + Lang_CXX03); + auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match( + FromTU, cxxRecordDecl(hasName("X"))); + + CXXRecordDecl *ImportedX = Import(FromX, Lang_CXX03); + EXPECT_FALSE(ImportedX); // X itself fails to import. + + // The bug: without the fix, a later, independent request to import X's + // type silently succeeds, returning the half-built X as if nothing had + // gone wrong, because ASTImporter::ImportedTypes was never scrubbed + // when X's own Decl import failed. + ASTImporter *Importer = findFromTU(FromX)->Importer.get(); + const Type *FromXTy = FromTU->getASTContext().getCanonicalTagType(FromX)->getTypePtr(); + ASSERT_TRUE(FromXTy); + Expected<const Type *> ToTyOrErr = Importer->Import(FromXTy); + EXPECT_FALSE(static_cast<bool>(ToTyOrErr)); + if (!ToTyOrErr) + llvm::consumeError(ToTyOrErr.takeError()); +} + // Check that an error propagates to the dependent AST nodes. // In the below code it means that an error in X should propagate to A. // And even to F since the containing A is erroneous. >From 58467bc1704e632749ab49029fbd55f84e0c1a22 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Wed, 5 Aug 2026 15:11:23 +0200 Subject: [PATCH 02/15] format --- clang/lib/AST/ASTImporter.cpp | 3 ++- clang/unittests/AST/ASTImporterTest.cpp | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 0b85636a06598..ffaca93b5d2ac 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -10041,7 +10041,8 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { // Also scrub the imported type mapping, if applicable. Import(Type*) can // cache a type mapping to a declaration that ultimately fails. if (const auto *FromTD = dyn_cast<TagDecl>(FromD)) - if (const Type *FromTy = getFromContext().getCanonicalTagType(FromTD).getTypePtr()) + if (const Type *FromTy = + getFromContext().getCanonicalTagType(FromTD).getTypePtr()) ImportedTypes.erase(FromTy); // ImportedDecls and ImportedFromDecls are not symmetric. It may happen diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 038b86edb40fd..d5fd9ec1d17d2 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -6636,7 +6636,8 @@ TEST_P(ErrorHandlingTest, ImportedTypeCacheIsInvalidatedOnFailure) { // gone wrong, because ASTImporter::ImportedTypes was never scrubbed // when X's own Decl import failed. ASTImporter *Importer = findFromTU(FromX)->Importer.get(); - const Type *FromXTy = FromTU->getASTContext().getCanonicalTagType(FromX)->getTypePtr(); + const Type *FromXTy = + FromTU->getASTContext().getCanonicalTagType(FromX)->getTypePtr(); ASSERT_TRUE(FromXTy); Expected<const Type *> ToTyOrErr = Importer->Import(FromXTy); EXPECT_FALSE(static_cast<bool>(ToTyOrErr)); >From 38015cd6fe3241b7a8f71287ce2954d6764aa0f4 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Wed, 5 Aug 2026 15:19:15 +0200 Subject: [PATCH 03/15] clean up lit test --- .../regression/lambda-import-corruption.cpp | 35 +++++-------------- 1 file changed, 8 insertions(+), 27 deletions(-) diff --git a/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp b/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp index 0ecc0a6c03662..ce9a1e355bf1f 100644 --- a/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp +++ b/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp @@ -2,19 +2,16 @@ // RUN: mkdir -p %t // RUN: split-file %s %t -// Pathological case: a lambda's closure (the anonymous class that -// implements its operator()) is created as a Decl before its members are -// imported. If a member unrelated to the eventual failure -- e.g. the -// implicit copy constructor, whose parameter type is `const ClosureType&` -// -- imports successfully first, that success permanently caches the -// closure's type as "imported" in ASTImporter::ImportedTypes, before the -// member that actually fails (here, operator(), due to an unsupported -// trailing requires-clause) is even reached. That cache is never +// Pathological case: a lambda's closure is created as a Decl before its +// members are imported. If a member unrelated to the eventual failure +// (e.g. the implicit copy constructor) imports successfully first, that +// success permanently caches the closure's type as "imported", before the +// member that actually fails is even reached. That cache was never // invalidated when the closure's own import later fails, so anything that -// subsequently needs the same type (the DeclRefExpr inside -// `decltype(func(...))` on `rudolf`, below) silently gets the half-built +// subsequently needs the same type (e.g. the DeclRefExpr inside +// `decltype(func(...))` on `rudolf`, below) silently got the half-built // closure back instead of a clean failure, producing an inconsistent node -// that crashes downstream. +// that crashed. // RUN: %clang_cc1 -std=c++20 -emit-pch -o %t/api.cpp.ast %t/api.cpp @@ -50,14 +47,6 @@ void entrypoint() { import_api(0); // [email protected]:20 {{Division by zero}} } -void trigger1() { - trigger_api(); -} - -void trigger2() { - trigger_isolate(); -} - //--- api.cpp template <class> int declval(); @@ -82,10 +71,6 @@ void import_api(int v) { import_isolate(v); } -void trigger_api() { - ns::rudolf<void>(0); // fails to import -} - //--- isolate.cpp template <class> int declval(); @@ -108,7 +93,3 @@ void import_isolate(int v) { (void)ns::import_ns; (void)(42 / v); // raises "Division by zero" } - -void trigger_isolate() { - ns::rudolf<void>(0); // fails to import -} >From 5c4f13e528d21b53069e5ab375409ecaa5e629fd Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Wed, 5 Aug 2026 15:20:49 +0200 Subject: [PATCH 04/15] clean up lit test --- clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp b/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp index ce9a1e355bf1f..2d81b9697bdf2 100644 --- a/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp +++ b/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp @@ -40,8 +40,6 @@ inline constexpr auto func = []<class T>(const T p) {}; } void import_api(int v); -void trigger_api(); -void trigger_isolate(); void entrypoint() { import_api(0); // [email protected]:20 {{Division by zero}} >From 117b33b4449ab664136df3892f459236044fa792 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Fri, 7 Aug 2026 18:05:23 +0200 Subject: [PATCH 05/15] Rework main comments and move lit test --- ...pp => invalid-lambda-type-equivalence.cpp} | 16 ++------ clang/unittests/AST/ASTImporterTest.cpp | 38 +++++++++---------- 2 files changed, 21 insertions(+), 33 deletions(-) rename clang/test/Analysis/ctu/{regression/lambda-import-corruption.cpp => invalid-lambda-type-equivalence.cpp} (74%) diff --git a/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp b/clang/test/Analysis/ctu/invalid-lambda-type-equivalence.cpp similarity index 74% rename from clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp rename to clang/test/Analysis/ctu/invalid-lambda-type-equivalence.cpp index 2d81b9697bdf2..eed5bd6362a3f 100644 --- a/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp +++ b/clang/test/Analysis/ctu/invalid-lambda-type-equivalence.cpp @@ -2,18 +2,6 @@ // RUN: mkdir -p %t // RUN: split-file %s %t -// Pathological case: a lambda's closure is created as a Decl before its -// members are imported. If a member unrelated to the eventual failure -// (e.g. the implicit copy constructor) imports successfully first, that -// success permanently caches the closure's type as "imported", before the -// member that actually fails is even reached. That cache was never -// invalidated when the closure's own import later fails, so anything that -// subsequently needs the same type (e.g. the DeclRefExpr inside -// `decltype(func(...))` on `rudolf`, below) silently got the half-built -// closure back instead of a clean failure, producing an inconsistent node -// that crashed. - - // RUN: %clang_cc1 -std=c++20 -emit-pch -o %t/api.cpp.ast %t/api.cpp // RUN: %clang_cc1 -std=c++20 -emit-pch -o %t/isolate.cpp.ast %t/isolate.cpp @@ -33,6 +21,8 @@ //--- main.cpp +// Check that importing 'api' and then 'isolate' does not cause crash. + namespace ns { inline constexpr auto func = []<class T>(const T p) {}; @@ -79,7 +69,7 @@ int import_ns; constexpr auto func = []<class T>(const T p) requires requires { 0; } {}; // Structural equivalence of the return type accesses the closure's -// definition through its type -- an access that assumes the closure is +// definition through its type, an access that assumes the closure is // intact. template <class K> decltype(func(declval<K>())) rudolf(int v) { // no-crash (void)(42 / v); diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index d5fd9ec1d17d2..31f76776ba12f 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -6608,20 +6608,22 @@ TEST_P(ErrorHandlingTest, ErrorIsPropagatedFromMemberToClass) { EXPECT_FALSE(ImportedOK); } -// A member whose signature refers back to the enclosing class (e.g. a -// copy constructor's `const Self&` parameter) can succeed and cache the -// class's *type* before a later, failing member causes the class's own -// Decl import to fail as a whole. Check that this doesn't leave a stale, -// "successfully imported" entry for the class's type behind: any later, -// independent request to import that type must also fail, not silently -// hand back the half-built class. -TEST_P(ErrorHandlingTest, ImportedTypeCacheIsInvalidatedOnFailure) { +// Check that the imported types, and not only the decls, are invalidated +// (removed from ImportedTypes) upon an import failure. It can happen, for +// instance with a member whose signature refers back to the enclosing class, +// that the type is successfully imported and pointing to the decl being +// imported, but that the decl import then fails further on. +// The decl mapping is correctly invalidated, but if the connected type is not +// invalidated as well, the half-built decl (which unavoidably remains +// in the 'To' AST) could be accessed through the type during later operations, +// like structural equivalence checks. +TEST_P(ErrorHandlingTest, ImportedTypeMappingIsInvalidatedOnFailure) { TranslationUnitDecl *FromTU = getTuDecl(std::string(R"( class X { - void ok(const X &) {} // Succeeds; imports X's own type - // as a side effect, before X's - // own import is known to fail. - void bad() { )") + ErroneousStmt + R"( } // Fails to import. + void ok(const X &) {} // Succeeds; imports X's own type + // as a side effect, before X's + // own import is known to fail. + void bad() { )") + ErroneousStmt + R"(} // Fails to import. }; )", Lang_CXX03); @@ -6629,20 +6631,16 @@ TEST_P(ErrorHandlingTest, ImportedTypeCacheIsInvalidatedOnFailure) { FromTU, cxxRecordDecl(hasName("X"))); CXXRecordDecl *ImportedX = Import(FromX, Lang_CXX03); - EXPECT_FALSE(ImportedX); // X itself fails to import. + // Class X fails to import + EXPECT_FALSE(ImportedX); - // The bug: without the fix, a later, independent request to import X's - // type silently succeeds, returning the half-built X as if nothing had - // gone wrong, because ASTImporter::ImportedTypes was never scrubbed - // when X's own Decl import failed. ASTImporter *Importer = findFromTU(FromX)->Importer.get(); const Type *FromXTy = FromTU->getASTContext().getCanonicalTagType(FromX)->getTypePtr(); ASSERT_TRUE(FromXTy); Expected<const Type *> ToTyOrErr = Importer->Import(FromXTy); - EXPECT_FALSE(static_cast<bool>(ToTyOrErr)); - if (!ToTyOrErr) - llvm::consumeError(ToTyOrErr.takeError()); + // And its type should fail to import as well + EXPECT_TRUE(ToTyOrErr.errorIsA<clang::ASTImportError>()); } // Check that an error propagates to the dependent AST nodes. >From 5243a5c8e90d0ee4a697e092fe81253a67d3f0ae Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Fri, 7 Aug 2026 18:13:39 +0200 Subject: [PATCH 06/15] Cast to TypeDecl instead --- clang/lib/AST/ASTImporter.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index ffaca93b5d2ac..2f4c7568be2f0 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -10038,12 +10038,10 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { auto *ToD = CreatedToD; ImportedDecls.erase(Pos); - // Also scrub the imported type mapping, if applicable. Import(Type*) can - // cache a type mapping to a declaration that ultimately fails. - if (const auto *FromTD = dyn_cast<TagDecl>(FromD)) - if (const Type *FromTy = - getFromContext().getCanonicalTagType(FromTD).getTypePtr()) - ImportedTypes.erase(FromTy); + // Scrub the imported type mapping as well. Import(Type*) can add a + // type mapping linked to a declaration that ultimately fails. + if (const auto *FromTD = dyn_cast<TypeDecl>(FromD)) + ImportedTypes.erase(FromTD->getTypeForDecl()); // ImportedDecls and ImportedFromDecls are not symmetric. It may happen // (e.g. with namespaces) that several decls from the 'from' context are >From 5242abdd8b4a2cb46c5c54fa2d2fad02ee4e95c4 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Fri, 7 Aug 2026 18:17:31 +0200 Subject: [PATCH 07/15] Remove additional potentially failure cases --- clang/lib/AST/ASTImporter.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 2f4c7568be2f0..29a16b7504eeb 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -10096,6 +10096,10 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { break; PrevFromDi = FromDi; setImportDeclError(FromDi, ErrOut); + + if (const auto *FromTDi = dyn_cast<TypeDecl>(FromDi)) + ImportedTypes.erase(FromTDi->getTypeForDecl()); + //FIXME Should we remove these Decls from ImportedDecls? // Set the error for the mapped to Decl, which is in the "to" context. auto Ii = ImportedDecls.find(FromDi); >From d0ca7cbcf7934fd01389e416e9ebd427ef4769d8 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Fri, 7 Aug 2026 18:22:34 +0200 Subject: [PATCH 08/15] format --- clang/lib/AST/ASTImporter.cpp | 4 ++-- clang/unittests/AST/ASTImporterTest.cpp | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 29a16b7504eeb..e96372e7c9523 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -10041,7 +10041,7 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { // Scrub the imported type mapping as well. Import(Type*) can add a // type mapping linked to a declaration that ultimately fails. if (const auto *FromTD = dyn_cast<TypeDecl>(FromD)) - ImportedTypes.erase(FromTD->getTypeForDecl()); + ImportedTypes.erase(FromTD->getTypeForDecl()); // ImportedDecls and ImportedFromDecls are not symmetric. It may happen // (e.g. with namespaces) that several decls from the 'from' context are @@ -10096,7 +10096,7 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { break; PrevFromDi = FromDi; setImportDeclError(FromDi, ErrOut); - + if (const auto *FromTDi = dyn_cast<TypeDecl>(FromDi)) ImportedTypes.erase(FromTDi->getTypeForDecl()); diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 31f76776ba12f..54c662dea94c4 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -6608,14 +6608,14 @@ TEST_P(ErrorHandlingTest, ErrorIsPropagatedFromMemberToClass) { EXPECT_FALSE(ImportedOK); } -// Check that the imported types, and not only the decls, are invalidated -// (removed from ImportedTypes) upon an import failure. It can happen, for -// instance with a member whose signature refers back to the enclosing class, +// Check that the imported types, and not only the decls, are invalidated +// (removed from ImportedTypes) upon an import failure. It can happen, for +// instance with a member whose signature refers back to the enclosing class, // that the type is successfully imported and pointing to the decl being -// imported, but that the decl import then fails further on. -// The decl mapping is correctly invalidated, but if the connected type is not -// invalidated as well, the half-built decl (which unavoidably remains -// in the 'To' AST) could be accessed through the type during later operations, +// imported, but that the decl import then fails further on. +// The decl mapping is correctly invalidated, but if the connected type is not +// invalidated as well, the half-built decl (which unavoidably remains +// in the 'To' AST) could be accessed through the type during later operations, // like structural equivalence checks. TEST_P(ErrorHandlingTest, ImportedTypeMappingIsInvalidatedOnFailure) { TranslationUnitDecl *FromTU = getTuDecl(std::string(R"( >From 1484941fdfb5f53bd89120a0d271fed3ac00f7dd Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Mon, 10 Aug 2026 09:11:39 +0200 Subject: [PATCH 09/15] Update clang/lib/AST/ASTImporter.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Balázs Kéri <[email protected]> --- clang/lib/AST/ASTImporter.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index e96372e7c9523..02c54e4d6f472 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -10038,8 +10038,8 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { auto *ToD = CreatedToD; ImportedDecls.erase(Pos); - // Scrub the imported type mapping as well. Import(Type*) can add a - // type mapping linked to a declaration that ultimately fails. + // Remove the imported type mapping as well. + // The imported type can point to a declaration that failed to import later. if (const auto *FromTD = dyn_cast<TypeDecl>(FromD)) ImportedTypes.erase(FromTD->getTypeForDecl()); >From e88c6390d808b2b6b1ff209ca2831d8b6e3b71f4 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Mon, 10 Aug 2026 09:51:43 +0200 Subject: [PATCH 10/15] format --- clang/lib/AST/ASTImporter.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index efc666608cc8e..df3d7b3f75905 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -10044,7 +10044,8 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { ImportedDecls.erase(Pos); // Remove the imported type mapping as well. - // The imported type can point to a declaration that failed to import later. + // The imported type can point to a declaration that failed to import + // later. if (const auto *FromTD = dyn_cast<TypeDecl>(FromD)) ImportedTypes.erase(FromTD->getTypeForDecl()); >From afef008fae5d5b5b8f7b72d77011a498870218f3 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Thu, 13 Aug 2026 11:08:20 +0200 Subject: [PATCH 11/15] Call right function --- clang/lib/AST/ASTImporter.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index e47f0040a6ee8..b81de25c237ea 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -9823,8 +9823,8 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { // Remove the imported type mapping as well. // The imported type can point to a declaration that failed to import // later. - if (const auto *FromTD = dyn_cast<TypeDecl>(FromD)) - ImportedTypes.erase(FromTD->getTypeForDecl()); + if (const auto *FromTD = dyn_cast<TagDecl>(FromD)) + ImportedTypes.erase(cast<TypeDecl>(FromTD)->getTypeForDecl()); // ImportedDecls and ImportedFromDecls are not symmetric. It may happen // (e.g. with namespaces) that several decls from the 'from' context are @@ -9880,8 +9880,8 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { PrevFromDi = FromDi; setImportDeclError(FromDi, ErrOut); - if (const auto *FromTDi = dyn_cast<TypeDecl>(FromDi)) - ImportedTypes.erase(FromTDi->getTypeForDecl()); + if (const auto *FromTDi = dyn_cast<TagDecl>(FromDi)) + ImportedTypes.erase(cast<TypeDecl>(FromTDi)->getTypeForDecl()); //FIXME Should we remove these Decls from ImportedDecls? // Set the error for the mapped to Decl, which is in the "to" context. >From d57d88c43ad3449c4c38688fd5a51f8ed2fb34d1 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Thu, 13 Aug 2026 11:21:17 +0200 Subject: [PATCH 12/15] Revert to initial version --- clang/lib/AST/ASTImporter.cpp | 253 ++++++++++++++++++++++++++++++++-- 1 file changed, 242 insertions(+), 11 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index b81de25c237ea..aff3aa968225f 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -755,8 +755,13 @@ namespace clang { Error ImportOverriddenMethods(CXXMethodDecl *ToMethod, CXXMethodDecl *FromMethod); - Expected<FunctionDecl *> - FindFunctionTemplateSpecialization(FunctionDecl *FromFD); + Expected<FunctionDecl *> FindFunctionTemplateSpecialization( + FunctionDecl *FromFD); + + // Returns true if the given function has a placeholder return type and + // that type is declared inside the body of the function. + // E.g. auto f() { struct X{}; return X(); } + bool hasReturnTypeDeclaredInside(FunctionDecl *D); }; template <typename InContainerTy> @@ -3776,6 +3781,223 @@ Error ASTNodeImporter::ImportFunctionDeclBody(FunctionDecl *FromFD, return Error::success(); } +// Returns true if the given D has a DeclContext up to the TranslationUnitDecl +// which is equal to the given DC, or D is equal to DC. +static bool isAncestorDeclContextOf(const DeclContext *DC, const Decl *D) { + const DeclContext *DCi = dyn_cast<DeclContext>(D); + if (!DCi) + DCi = D->getDeclContext(); + assert(DCi && "Declaration should have a context"); + while (DCi != D->getTranslationUnitDecl()) { + if (DCi == DC) + return true; + DCi = DCi->getParent(); + } + return false; +} + +// Check if there is a declaration that has 'DC' as parent context and is +// referenced from statement 'S' or one of its children. The search is done in +// BFS order through children of 'S'. +static bool isAncestorDeclContextOf(const DeclContext *DC, const Stmt *S) { + SmallVector<const Stmt *> ToProcess; + ToProcess.push_back(S); + while (!ToProcess.empty()) { + const Stmt *CurrentS = ToProcess.pop_back_val(); + ToProcess.append(CurrentS->child_begin(), CurrentS->child_end()); + if (const auto *DeclRef = dyn_cast<DeclRefExpr>(CurrentS)) { + if (const Decl *D = DeclRef->getDecl()) + if (isAncestorDeclContextOf(DC, D)) + return true; + } else if (const auto *E = + dyn_cast_or_null<SubstNonTypeTemplateParmExpr>(CurrentS)) { + if (const Decl *D = E->getAssociatedDecl()) + if (isAncestorDeclContextOf(DC, D)) + return true; + } + } + return false; +} + +namespace { +/// Check if a type has any reference to a declaration that is inside the body +/// of a function. +/// The \c CheckType(QualType) function should be used to determine +/// this property. +/// +/// The type visitor visits one type object only (not recursive). +/// To find all referenced declarations we must discover all type objects until +/// the canonical type is reached (walk over typedef and similar objects). This +/// is done by loop over all "sugar" type objects. For every such type we must +/// check all declarations that are referenced from it. For this check the +/// visitor is used. In the visit functions all referenced declarations except +/// the one that follows in the sugar chain (if any) must be checked. For this +/// check the same visitor is re-used (it has no state-dependent data). +/// +/// The visit functions have 3 possible return values: +/// - True, found a declaration inside \c ParentDC. +/// - False, found declarations only outside \c ParentDC and it is not possible +/// to find more declarations (the "sugar" chain does not continue). +/// - Empty optional value, found no declarations or only outside \c ParentDC, +/// but it is possible to find more declarations in the type "sugar" chain. +/// The loop over the "sugar" types can be implemented by using type visit +/// functions only (call \c CheckType with the desugared type). With the current +/// solution no visit function is needed if the type has only a desugared type +/// as data. +class IsTypeDeclaredInsideVisitor + : public TypeVisitor<IsTypeDeclaredInsideVisitor, std::optional<bool>> { +public: + IsTypeDeclaredInsideVisitor(const FunctionDecl *ParentDC) + : ParentDC(ParentDC) {} + + bool CheckType(QualType T) { + // Check the chain of "sugar" types. + // The "sugar" types are typedef or similar types that have the same + // canonical type. + if (std::optional<bool> Res = Visit(T.getTypePtr())) + return *Res; + QualType DsT = + T.getSingleStepDesugaredType(ParentDC->getParentASTContext()); + while (DsT != T) { + if (std::optional<bool> Res = Visit(DsT.getTypePtr())) + return *Res; + T = DsT; + DsT = T.getSingleStepDesugaredType(ParentDC->getParentASTContext()); + } + return false; + } + + std::optional<bool> VisitTagType(const TagType *T) { + if (auto *Spec = dyn_cast<ClassTemplateSpecializationDecl>(T->getDecl())) + for (const auto &Arg : Spec->getTemplateArgs().asArray()) + if (checkTemplateArgument(Arg)) + return true; + return isAncestorDeclContextOf(ParentDC, T->getDecl()); + } + + std::optional<bool> VisitPointerType(const PointerType *T) { + return CheckType(T->getPointeeType()); + } + + std::optional<bool> VisitReferenceType(const ReferenceType *T) { + return CheckType(T->getPointeeTypeAsWritten()); + } + + std::optional<bool> VisitTypedefType(const TypedefType *T) { + return isAncestorDeclContextOf(ParentDC, T->getDecl()); + } + + std::optional<bool> VisitUsingType(const UsingType *T) { + return isAncestorDeclContextOf(ParentDC, T->getDecl()); + } + + std::optional<bool> + VisitTemplateSpecializationType(const TemplateSpecializationType *T) { + for (const auto &Arg : T->template_arguments()) + if (checkTemplateArgument(Arg)) + return true; + // This type is a "sugar" to a record type, it can have a desugared type. + return {}; + } + + std::optional<bool> VisitUnaryTransformType(const UnaryTransformType *T) { + return CheckType(T->getBaseType()); + } + + std::optional<bool> + VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *T) { + // The "associated declaration" can be the same as ParentDC. + if (isAncestorDeclContextOf(ParentDC, T->getAssociatedDecl())) + return true; + return {}; + } + + std::optional<bool> VisitConstantArrayType(const ConstantArrayType *T) { + if (T->getSizeExpr() && isAncestorDeclContextOf(ParentDC, T->getSizeExpr())) + return true; + + return CheckType(T->getElementType()); + } + + std::optional<bool> VisitVariableArrayType(const VariableArrayType *T) { + llvm_unreachable( + "Variable array should not occur in deduced return type of a function"); + } + + std::optional<bool> VisitIncompleteArrayType(const IncompleteArrayType *T) { + llvm_unreachable("Incomplete array should not occur in deduced return type " + "of a function"); + } + + std::optional<bool> VisitDependentArrayType(const IncompleteArrayType *T) { + llvm_unreachable("Dependent array should not occur in deduced return type " + "of a function"); + } + +private: + const DeclContext *const ParentDC; + + bool checkTemplateArgument(const TemplateArgument &Arg) { + switch (Arg.getKind()) { + case TemplateArgument::Null: + return false; + case TemplateArgument::Integral: + return CheckType(Arg.getIntegralType()); + case TemplateArgument::Type: + return CheckType(Arg.getAsType()); + case TemplateArgument::Expression: + return isAncestorDeclContextOf(ParentDC, Arg.getAsExpr()); + case TemplateArgument::Declaration: + // FIXME: The declaration in this case is not allowed to be in a function? + return isAncestorDeclContextOf(ParentDC, Arg.getAsDecl()); + case TemplateArgument::NullPtr: + // FIXME: The type is not allowed to be in the function? + return CheckType(Arg.getNullPtrType()); + case TemplateArgument::StructuralValue: + return CheckType(Arg.getStructuralValueType()); + case TemplateArgument::Pack: + for (const auto &PackArg : Arg.getPackAsArray()) + if (checkTemplateArgument(PackArg)) + return true; + return false; + case TemplateArgument::Template: + // Templates can not be defined locally in functions. + // A template passed as argument can be not in ParentDC. + return false; + case TemplateArgument::TemplateExpansion: + // Templates can not be defined locally in functions. + // A template passed as argument can be not in ParentDC. + return false; + } + llvm_unreachable("Unknown TemplateArgument::ArgKind enum"); + }; +}; +} // namespace + +/// This function checks if the given function has a return type that contains +/// a reference (in any way) to a declaration inside the same function. +bool ASTNodeImporter::hasReturnTypeDeclaredInside(FunctionDecl *D) { + QualType FromTy = D->getType(); + const auto *FromFPT = FromTy->getAs<FunctionProtoType>(); + assert(FromFPT && "Must be called on FunctionProtoType"); + + auto IsCXX11Lambda = [&]() { + if (Importer.FromContext.getLangOpts().CPlusPlus14) // C++14 or later + return false; + + return isLambdaMethod(D); + }; + + QualType RetT = FromFPT->getReturnType(); + if (isa<AutoType>(RetT.getTypePtr()) || IsCXX11Lambda()) { + FunctionDecl *Def = D->getDefinition(); + IsTypeDeclaredInsideVisitor Visitor(Def ? Def : D); + return Visitor.CheckType(RetT); + } + + return false; +} + ExplicitSpecifier ASTNodeImporter::importExplicitSpecifier(Error &Err, ExplicitSpecifier ESpec) { Expr *ExplicitExpr = ESpec.getExpr(); @@ -3921,11 +4143,12 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { // Functions with auto return type may define a struct inside their body // and the return type could refer to that struct. // E.g.: auto foo() { struct X{}; return X(); } - // There are many more cases when types inside the function declaration - // can appear in the return type, like types declared as typenames from - // template params. - // All such cases are tracked in FindFunctionDeclImportCycle. - if (Importer.FindFunctionDeclImportCycle.isCycle(D)) { + // To avoid an infinite recursion when importing, create the FunctionDecl + // with a simplified return type. + // Reuse this approach for auto return types declared as typenames from + // template params, tracked in FindFunctionDeclImportCycle. + if (hasReturnTypeDeclaredInside(D) || + Importer.FindFunctionDeclImportCycle.isCycle(D)) { FromReturnTy = Importer.getFromContext().VoidTy; UsedDifferentProtoType = true; } @@ -9823,8 +10046,12 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { // Remove the imported type mapping as well. // The imported type can point to a declaration that failed to import // later. - if (const auto *FromTD = dyn_cast<TagDecl>(FromD)) - ImportedTypes.erase(cast<TypeDecl>(FromTD)->getTypeForDecl()); + if (const auto *FromTD = dyn_cast<TagDecl>(FromD)) { + if (const Type *FromTy = + getFromContext().getCanonicalTagType(FromTD).getTypePtr()) { + ImportedTypes.erase(FromTy); + } + } // ImportedDecls and ImportedFromDecls are not symmetric. It may happen // (e.g. with namespaces) that several decls from the 'from' context are @@ -9880,8 +10107,12 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { PrevFromDi = FromDi; setImportDeclError(FromDi, ErrOut); - if (const auto *FromTDi = dyn_cast<TagDecl>(FromDi)) - ImportedTypes.erase(cast<TypeDecl>(FromTDi)->getTypeForDecl()); + if (const auto *FromTD = dyn_cast<TagDecl>(FromDi)) { + if (const Type *FromTy = + getFromContext().getCanonicalTagType(FromTD).getTypePtr()) { + ImportedTypes.erase(FromTy); + } + } //FIXME Should we remove these Decls from ImportedDecls? // Set the error for the mapped to Decl, which is in the "to" context. >From 7ab789250eeac16986e6702f7f26dab410cb436d Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Thu, 13 Aug 2026 11:22:36 +0200 Subject: [PATCH 13/15] Revert "Revert to initial version" This reverts commit d57d88c43ad3449c4c38688fd5a51f8ed2fb34d1. --- clang/lib/AST/ASTImporter.cpp | 253 ++-------------------------------- 1 file changed, 11 insertions(+), 242 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index aff3aa968225f..b81de25c237ea 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -755,13 +755,8 @@ namespace clang { Error ImportOverriddenMethods(CXXMethodDecl *ToMethod, CXXMethodDecl *FromMethod); - Expected<FunctionDecl *> FindFunctionTemplateSpecialization( - FunctionDecl *FromFD); - - // Returns true if the given function has a placeholder return type and - // that type is declared inside the body of the function. - // E.g. auto f() { struct X{}; return X(); } - bool hasReturnTypeDeclaredInside(FunctionDecl *D); + Expected<FunctionDecl *> + FindFunctionTemplateSpecialization(FunctionDecl *FromFD); }; template <typename InContainerTy> @@ -3781,223 +3776,6 @@ Error ASTNodeImporter::ImportFunctionDeclBody(FunctionDecl *FromFD, return Error::success(); } -// Returns true if the given D has a DeclContext up to the TranslationUnitDecl -// which is equal to the given DC, or D is equal to DC. -static bool isAncestorDeclContextOf(const DeclContext *DC, const Decl *D) { - const DeclContext *DCi = dyn_cast<DeclContext>(D); - if (!DCi) - DCi = D->getDeclContext(); - assert(DCi && "Declaration should have a context"); - while (DCi != D->getTranslationUnitDecl()) { - if (DCi == DC) - return true; - DCi = DCi->getParent(); - } - return false; -} - -// Check if there is a declaration that has 'DC' as parent context and is -// referenced from statement 'S' or one of its children. The search is done in -// BFS order through children of 'S'. -static bool isAncestorDeclContextOf(const DeclContext *DC, const Stmt *S) { - SmallVector<const Stmt *> ToProcess; - ToProcess.push_back(S); - while (!ToProcess.empty()) { - const Stmt *CurrentS = ToProcess.pop_back_val(); - ToProcess.append(CurrentS->child_begin(), CurrentS->child_end()); - if (const auto *DeclRef = dyn_cast<DeclRefExpr>(CurrentS)) { - if (const Decl *D = DeclRef->getDecl()) - if (isAncestorDeclContextOf(DC, D)) - return true; - } else if (const auto *E = - dyn_cast_or_null<SubstNonTypeTemplateParmExpr>(CurrentS)) { - if (const Decl *D = E->getAssociatedDecl()) - if (isAncestorDeclContextOf(DC, D)) - return true; - } - } - return false; -} - -namespace { -/// Check if a type has any reference to a declaration that is inside the body -/// of a function. -/// The \c CheckType(QualType) function should be used to determine -/// this property. -/// -/// The type visitor visits one type object only (not recursive). -/// To find all referenced declarations we must discover all type objects until -/// the canonical type is reached (walk over typedef and similar objects). This -/// is done by loop over all "sugar" type objects. For every such type we must -/// check all declarations that are referenced from it. For this check the -/// visitor is used. In the visit functions all referenced declarations except -/// the one that follows in the sugar chain (if any) must be checked. For this -/// check the same visitor is re-used (it has no state-dependent data). -/// -/// The visit functions have 3 possible return values: -/// - True, found a declaration inside \c ParentDC. -/// - False, found declarations only outside \c ParentDC and it is not possible -/// to find more declarations (the "sugar" chain does not continue). -/// - Empty optional value, found no declarations or only outside \c ParentDC, -/// but it is possible to find more declarations in the type "sugar" chain. -/// The loop over the "sugar" types can be implemented by using type visit -/// functions only (call \c CheckType with the desugared type). With the current -/// solution no visit function is needed if the type has only a desugared type -/// as data. -class IsTypeDeclaredInsideVisitor - : public TypeVisitor<IsTypeDeclaredInsideVisitor, std::optional<bool>> { -public: - IsTypeDeclaredInsideVisitor(const FunctionDecl *ParentDC) - : ParentDC(ParentDC) {} - - bool CheckType(QualType T) { - // Check the chain of "sugar" types. - // The "sugar" types are typedef or similar types that have the same - // canonical type. - if (std::optional<bool> Res = Visit(T.getTypePtr())) - return *Res; - QualType DsT = - T.getSingleStepDesugaredType(ParentDC->getParentASTContext()); - while (DsT != T) { - if (std::optional<bool> Res = Visit(DsT.getTypePtr())) - return *Res; - T = DsT; - DsT = T.getSingleStepDesugaredType(ParentDC->getParentASTContext()); - } - return false; - } - - std::optional<bool> VisitTagType(const TagType *T) { - if (auto *Spec = dyn_cast<ClassTemplateSpecializationDecl>(T->getDecl())) - for (const auto &Arg : Spec->getTemplateArgs().asArray()) - if (checkTemplateArgument(Arg)) - return true; - return isAncestorDeclContextOf(ParentDC, T->getDecl()); - } - - std::optional<bool> VisitPointerType(const PointerType *T) { - return CheckType(T->getPointeeType()); - } - - std::optional<bool> VisitReferenceType(const ReferenceType *T) { - return CheckType(T->getPointeeTypeAsWritten()); - } - - std::optional<bool> VisitTypedefType(const TypedefType *T) { - return isAncestorDeclContextOf(ParentDC, T->getDecl()); - } - - std::optional<bool> VisitUsingType(const UsingType *T) { - return isAncestorDeclContextOf(ParentDC, T->getDecl()); - } - - std::optional<bool> - VisitTemplateSpecializationType(const TemplateSpecializationType *T) { - for (const auto &Arg : T->template_arguments()) - if (checkTemplateArgument(Arg)) - return true; - // This type is a "sugar" to a record type, it can have a desugared type. - return {}; - } - - std::optional<bool> VisitUnaryTransformType(const UnaryTransformType *T) { - return CheckType(T->getBaseType()); - } - - std::optional<bool> - VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *T) { - // The "associated declaration" can be the same as ParentDC. - if (isAncestorDeclContextOf(ParentDC, T->getAssociatedDecl())) - return true; - return {}; - } - - std::optional<bool> VisitConstantArrayType(const ConstantArrayType *T) { - if (T->getSizeExpr() && isAncestorDeclContextOf(ParentDC, T->getSizeExpr())) - return true; - - return CheckType(T->getElementType()); - } - - std::optional<bool> VisitVariableArrayType(const VariableArrayType *T) { - llvm_unreachable( - "Variable array should not occur in deduced return type of a function"); - } - - std::optional<bool> VisitIncompleteArrayType(const IncompleteArrayType *T) { - llvm_unreachable("Incomplete array should not occur in deduced return type " - "of a function"); - } - - std::optional<bool> VisitDependentArrayType(const IncompleteArrayType *T) { - llvm_unreachable("Dependent array should not occur in deduced return type " - "of a function"); - } - -private: - const DeclContext *const ParentDC; - - bool checkTemplateArgument(const TemplateArgument &Arg) { - switch (Arg.getKind()) { - case TemplateArgument::Null: - return false; - case TemplateArgument::Integral: - return CheckType(Arg.getIntegralType()); - case TemplateArgument::Type: - return CheckType(Arg.getAsType()); - case TemplateArgument::Expression: - return isAncestorDeclContextOf(ParentDC, Arg.getAsExpr()); - case TemplateArgument::Declaration: - // FIXME: The declaration in this case is not allowed to be in a function? - return isAncestorDeclContextOf(ParentDC, Arg.getAsDecl()); - case TemplateArgument::NullPtr: - // FIXME: The type is not allowed to be in the function? - return CheckType(Arg.getNullPtrType()); - case TemplateArgument::StructuralValue: - return CheckType(Arg.getStructuralValueType()); - case TemplateArgument::Pack: - for (const auto &PackArg : Arg.getPackAsArray()) - if (checkTemplateArgument(PackArg)) - return true; - return false; - case TemplateArgument::Template: - // Templates can not be defined locally in functions. - // A template passed as argument can be not in ParentDC. - return false; - case TemplateArgument::TemplateExpansion: - // Templates can not be defined locally in functions. - // A template passed as argument can be not in ParentDC. - return false; - } - llvm_unreachable("Unknown TemplateArgument::ArgKind enum"); - }; -}; -} // namespace - -/// This function checks if the given function has a return type that contains -/// a reference (in any way) to a declaration inside the same function. -bool ASTNodeImporter::hasReturnTypeDeclaredInside(FunctionDecl *D) { - QualType FromTy = D->getType(); - const auto *FromFPT = FromTy->getAs<FunctionProtoType>(); - assert(FromFPT && "Must be called on FunctionProtoType"); - - auto IsCXX11Lambda = [&]() { - if (Importer.FromContext.getLangOpts().CPlusPlus14) // C++14 or later - return false; - - return isLambdaMethod(D); - }; - - QualType RetT = FromFPT->getReturnType(); - if (isa<AutoType>(RetT.getTypePtr()) || IsCXX11Lambda()) { - FunctionDecl *Def = D->getDefinition(); - IsTypeDeclaredInsideVisitor Visitor(Def ? Def : D); - return Visitor.CheckType(RetT); - } - - return false; -} - ExplicitSpecifier ASTNodeImporter::importExplicitSpecifier(Error &Err, ExplicitSpecifier ESpec) { Expr *ExplicitExpr = ESpec.getExpr(); @@ -4143,12 +3921,11 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { // Functions with auto return type may define a struct inside their body // and the return type could refer to that struct. // E.g.: auto foo() { struct X{}; return X(); } - // To avoid an infinite recursion when importing, create the FunctionDecl - // with a simplified return type. - // Reuse this approach for auto return types declared as typenames from - // template params, tracked in FindFunctionDeclImportCycle. - if (hasReturnTypeDeclaredInside(D) || - Importer.FindFunctionDeclImportCycle.isCycle(D)) { + // There are many more cases when types inside the function declaration + // can appear in the return type, like types declared as typenames from + // template params. + // All such cases are tracked in FindFunctionDeclImportCycle. + if (Importer.FindFunctionDeclImportCycle.isCycle(D)) { FromReturnTy = Importer.getFromContext().VoidTy; UsedDifferentProtoType = true; } @@ -10046,12 +9823,8 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { // Remove the imported type mapping as well. // The imported type can point to a declaration that failed to import // later. - if (const auto *FromTD = dyn_cast<TagDecl>(FromD)) { - if (const Type *FromTy = - getFromContext().getCanonicalTagType(FromTD).getTypePtr()) { - ImportedTypes.erase(FromTy); - } - } + if (const auto *FromTD = dyn_cast<TagDecl>(FromD)) + ImportedTypes.erase(cast<TypeDecl>(FromTD)->getTypeForDecl()); // ImportedDecls and ImportedFromDecls are not symmetric. It may happen // (e.g. with namespaces) that several decls from the 'from' context are @@ -10107,12 +9880,8 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { PrevFromDi = FromDi; setImportDeclError(FromDi, ErrOut); - if (const auto *FromTD = dyn_cast<TagDecl>(FromDi)) { - if (const Type *FromTy = - getFromContext().getCanonicalTagType(FromTD).getTypePtr()) { - ImportedTypes.erase(FromTy); - } - } + if (const auto *FromTDi = dyn_cast<TagDecl>(FromDi)) + ImportedTypes.erase(cast<TypeDecl>(FromTDi)->getTypeForDecl()); //FIXME Should we remove these Decls from ImportedDecls? // Set the error for the mapped to Decl, which is in the "to" context. >From 0e9de397117e58da74261def278ea35f084a6c81 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Thu, 13 Aug 2026 11:24:38 +0200 Subject: [PATCH 14/15] Fix revert --- clang/lib/AST/ASTImporter.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index b81de25c237ea..01c5385ff5bb5 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -9823,8 +9823,12 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { // Remove the imported type mapping as well. // The imported type can point to a declaration that failed to import // later. - if (const auto *FromTD = dyn_cast<TagDecl>(FromD)) - ImportedTypes.erase(cast<TypeDecl>(FromTD)->getTypeForDecl()); + if (const auto *FromTD = dyn_cast<TagDecl>(FromD)) { + if (const Type *FromTy = + getFromContext().getCanonicalTagType(FromTD).getTypePtr()) { + ImportedTypes.erase(FromTy); + } + } // ImportedDecls and ImportedFromDecls are not symmetric. It may happen // (e.g. with namespaces) that several decls from the 'from' context are @@ -9880,8 +9884,12 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { PrevFromDi = FromDi; setImportDeclError(FromDi, ErrOut); - if (const auto *FromTDi = dyn_cast<TagDecl>(FromDi)) - ImportedTypes.erase(cast<TypeDecl>(FromTDi)->getTypeForDecl()); + if (const auto *FromTDi = dyn_cast<TagDecl>(FromDi)) { + if (const Type *FromTyi = + getFromContext().getCanonicalTagType(FromTDi).getTypePtr()) { + ImportedTypes.erase(FromTyi); + } + } //FIXME Should we remove these Decls from ImportedDecls? // Set the error for the mapped to Decl, which is in the "to" context. >From 78324320e68e62159faaaa5e47c586edf3e47c42 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Thu, 13 Aug 2026 12:18:32 +0200 Subject: [PATCH 15/15] Consume error --- clang/unittests/AST/ASTImporterTest.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 54c662dea94c4..f3b4c9ca7fa9a 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -6641,6 +6641,7 @@ TEST_P(ErrorHandlingTest, ImportedTypeMappingIsInvalidatedOnFailure) { Expected<const Type *> ToTyOrErr = Importer->Import(FromXTy); // And its type should fail to import as well EXPECT_TRUE(ToTyOrErr.errorIsA<clang::ASTImportError>()); + llvm::consumeError(ToTyOrErr.takeError()); } // Check that an error propagates to the dependent AST nodes. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
