- Notifications
You must be signed in to change notification settings - Fork 15.3k
[clang] Propagate definition data to all redecls #170090
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
base: main
Are you sure you want to change the base?
Conversation
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
| @llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-modules Author: Jonas Hahnfeld (hahnjo) ChangesFix 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 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:
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 = ⅅ + for (auto *D : Def->redecls()) + cast<CXXRecordDecl>(D)->DefinitionData = ⅅ 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(); } |
vgvassilev left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
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