Skip to content

Conversation

@hahnjo
Copy link
Member

@hahnjo hahnjo commented Dec 1, 2025

Fix the propagation added in commit 0d490ae to include all redecls, not only previous ones. This fixes another instance of the assertion "Cannot get layout of forward declarations" in getASTRecordLayout().

Kudos to Alexander Kornienko for providing an initial version of the reproducer that I further simplified.

Fixes #170084

Fix the propagation added in commit 0d490ae to include all redecls, not only previous ones. This fixes another instance of the assertion "Cannot get layout of forward declarations" in getASTRecordLayout(). Kudos to Alexander Kornienko for providing an initial version of the reproducer that I further simplified. Fixes llvm#170084
@hahnjo hahnjo self-assigned this Dec 1, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:modules C++20 modules and Clang Header Modules labels Dec 1, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 1, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Jonas Hahnfeld (hahnjo)

Changes

Fix the propagation added in commit 0d490ae to include all redecls, not only previous ones. This fixes another instance of the assertion "Cannot get layout of forward declarations" in getASTRecordLayout().

Kudos to Alexander Kornienko for providing an initial version of the reproducer that I further simplified.

Fixes #170084


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

2 Files Affected:

  • (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+2-2)
  • (added) clang/test/Modules/GH170084.cpp (+75)
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 5456e73956659..882d54f31280a 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -2107,8 +2107,8 @@ void ASTDeclMerger::MergeDefinitionData( auto *Def = DD.Definition; DD = std::move(MergeDD); DD.Definition = Def; - while ((Def = Def->getPreviousDecl())) - cast<CXXRecordDecl>(Def)->DefinitionData = &DD; + for (auto *D : Def->redecls()) + cast<CXXRecordDecl>(D)->DefinitionData = &DD; return; } diff --git a/clang/test/Modules/GH170084.cpp b/clang/test/Modules/GH170084.cpp new file mode 100644 index 0000000000000..950499467a6bb --- /dev/null +++ b/clang/test/Modules/GH170084.cpp @@ -0,0 +1,75 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// RUN: cd %t + +// RUN: %clang_cc1 -fmodule-name=stl -fno-cxx-modules -emit-module -fmodules -xc++ stl.cppmap -o stl.pcm +// RUN: %clang_cc1 -fmodule-name=d -fno-cxx-modules -emit-module -fmodules -fmodule-file=stl.pcm -xc++ d.cppmap -o d.pcm +// RUN: %clang_cc1 -fmodule-name=b -fno-cxx-modules -emit-module -fmodules -fmodule-file=stl.pcm -xc++ b.cppmap -o b.pcm +// RUN: %clang_cc1 -fmodule-name=a -fno-cxx-modules -emit-module -fmodules -fmodule-file=stl.pcm -fmodule-file=d.pcm -fmodule-file=b.pcm -xc++ a.cppmap -o a.pcm +// RUN: %clang_cc1 -fno-cxx-modules -fmodules -fmodule-file=a.pcm -emit-llvm -o /dev/null main.cpp + +//--- a.cppmap +module "a" { +header "a.h" +} + +//--- a.h +#include "b.h" +namespace { +void a(absl::set<char> c) { + absl::set<int> b; + c.end(); + c.contains(); +} +} // namespace + +//--- b.cppmap +module "b" { +header "b.h" +} + +//--- b.h +#include "c.h" +void b() { absl::set<char> x; } + +//--- c.h +#include "stl.h" +namespace absl { +template <typename> +class set { + public: + struct iterator { + void u() const; + }; + iterator end() const { return {}; } + void contains() const { end().u(); } + pair<iterator> e(); +}; +} // namespace absl + +//--- d.cppmap +module "d" { +header "d.h" +} + +//--- d.h +#include "c.h" +void d() { absl::set<char> x; } + +//--- stl.cppmap +module "stl" { +header "stl.h" +} + +//--- stl.h +#ifndef _STL_H_ +#define _STL_H_ +template <class> +struct pair; +#endif + +//--- main.cpp +// expected-no-diagnostics +#include "c.h" +void f(absl::set<char> o) { o.contains(); } 
Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category

4 participants