[Lldb-commits] [clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)
@@ -9180,6 +9180,12 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D,
NamedDecl **Suggested,
if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
return true;
+ // The external source may have additional definitions of this entity that
are
+ // visible, so complete the redeclaration chain now.
+ if (auto *Source = Context.getExternalSource()) {
+Source->CompleteRedeclChain(D);
+ }
mpark wrote:
Further findings: in this case, what I'm seeing is that
[`LazyGenerationalUpdatePtr::get`](https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/ExternalASTSource.h#L487-L497)
is called twice for [4].
The first invocation, the generation numbers don't match, and
`CompleteRedeclChain` is called (*outside* of the deserialization stage) and
gets to
[`DC->lookup(Name);`](https://github.com/llvm/llvm-project/blob/c6d95c441a29a45782ff72d6cb82839b86fd0e4a/clang/lib/Serialization/ASTReader.cpp#L7862).
We don't discover the definition in the first invocation.
The second invocation is from
[here](https://github.com/llvm/llvm-project/blob/c6d95c441a29a45782ff72d6cb82839b86fd0e4a/clang/lib/Sema/SemaType.cpp#L9174)
(via the `getMostRecentDecl()` call within `getDefinition`). This time, the
generation numbers **match**, and `CompleteRedeclChain` is **not** called.
However, if I explicitly invoke `CompleteRedeclChain` on it, it gets to the
same `DC->lookup(Name);` line as before with exactly the same values for `DC`
and `Name`. However, this time the definition **is** found. To be clear, this
is invoking `CompleteRedeclChain` on [4] **directly** rather than on [1], [2]
or [3], and also not **indirectly** via something like `getMostRecentDecl()` or
`redecls()` since `CompleteRedeclChain` will not be called through those
functions because the generation numbers match.
In short, I'm seeing that the `DC->lookup(Name)` has different behaviors
between two invocations, without either generation numbers changing. I wonder
if maybe another module was loaded in between the two calls, and the
`ExternalASTSource`'s generation number should have been incremented but it
wasn't for some reason...
https://github.com/llvm/llvm-project/pull/129982
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [C++20][Modules] Do not update the generation number if the redeclaration chain completion was delayed. (PR #129982)
https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)
https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)
https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)
@@ -9180,6 +9180,12 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D,
NamedDecl **Suggested,
if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
return true;
+ // The external source may have additional definitions of this entity that
are
+ // visible, so complete the redeclaration chain now.
+ if (auto *Source = Context.getExternalSource()) {
+Source->CompleteRedeclChain(D);
+ }
mpark wrote:
On one thought, it seems like the decls inside `DC` should be notified of the
fact that there have been new additions made to `Lookups[DC]`... but on the
other hand, it's not clear to me whether this is an expected sequence of events
in the first place. If this is an expected sequence of events, I'm a bit
surprised that this doesn't seem to be a bigger issue. If this is not an
expected sequence of events, do we expect that the first call to
`CompleteRedeclChain` should have the `Lookups` populated with everything
already?
CC @ChuanqiXu9, maybe @Bigcheese can help here too?
https://github.com/llvm/llvm-project/pull/129982
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)
https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)
https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)
@@ -9180,6 +9180,12 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D,
NamedDecl **Suggested,
if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
return true;
+ // The external source may have additional definitions of this entity that
are
+ // visible, so complete the redeclaration chain now.
+ if (auto *Source = Context.getExternalSource()) {
+Source->CompleteRedeclChain(D);
+ }
mpark wrote:
Hm, okay... so the `DC->lookup(Name)` have different behaviors because of
[`Source->FindExternalVisibleDeclsByName(...)`](https://github.com/llvm/llvm-project/blob/release/20.x/clang/lib/AST/DeclBase.cpp#L1913).
In `ASTReader::FindExternalVisibleDeclsByName`, we get to
[here](https://github.com/llvm/llvm-project/blob/c6d95c441a29a45782ff72d6cb82839b86fd0e4a/clang/lib/Serialization/ASTReader.cpp#L8494-L8501)
looking through `Lookups` data structure. In the first call,
`Lookups[DC].Table[Name]` has 1 entry (just a decl), whereas in the second call
it has 2 entries (the decl and a defn).
In summary, we have `LazyGenerationalUpdatePtr::get` calling
`CompleteRedeclChain` where `Lookups` isn't populated with the definition, and
updates the generation number. The second time `LazyGenerationUpdatePtr::get`
is called, `Lookups` **is** populated with the definition, but the generation
number matches, and so it doesn't call `CompleteRedeclChain` even though it
would yield a different result.
https://github.com/llvm/llvm-project/pull/129982
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)
https://github.com/mpark converted_to_draft https://github.com/llvm/llvm-project/pull/129982 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [C++20][Modules] Fix incomplete decl chains (PR #129982)
https://github.com/mpark updated
https://github.com/llvm/llvm-project/pull/129982
>From 39b845fd64c2ce173b95ddcafbbab2d5116aa68e Mon Sep 17 00:00:00 2001
From: Michael Park
Date: Wed, 5 Mar 2025 18:46:10 -0800
Subject: [PATCH 1/3] [clang][modules] Add a unit test for the assertion
failure in bootstrap build on OS X with XCode.
---
clang/test/Modules/pr129982.cpp | 107
1 file changed, 107 insertions(+)
create mode 100644 clang/test/Modules/pr129982.cpp
diff --git a/clang/test/Modules/pr129982.cpp b/clang/test/Modules/pr129982.cpp
new file mode 100644
index 0..b19cccfd07687
--- /dev/null
+++ b/clang/test/Modules/pr129982.cpp
@@ -0,0 +1,107 @@
+// If this test fails, it should be investigated under Debug builds.
+// Before the PR, this test was violating an assertion.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -fmodules \
+// RUN: -fmodule-map-file=%t/module.modulemap \
+// RUN: -fmodules-cache-path=%t %t/a.cpp
+
+//--- module.modulemap
+module ebo {
+ header "ebo.h"
+}
+
+module fwd {
+ header "fwd.h"
+}
+
+module s {
+ header "s.h"
+ export *
+}
+
+module mod {
+ header "a.h"
+ header "b.h"
+}
+
+//--- ebo.h
+#pragma once
+
+namespace N { inline namespace __1 {
+
+template
+struct EBO : T {
+ EBO() = default;
+};
+
+}}
+
+//--- fwd.h
+#pragma once
+
+namespace N { inline namespace __1 {
+
+template
+struct Empty;
+
+template
+struct BS;
+
+using S = BS>;
+
+}}
+
+//--- s.h
+#pragma once
+
+#include "fwd.h"
+#include "ebo.h"
+
+namespace N { inline namespace __1 {
+
+template
+struct Empty {};
+
+template
+struct BS {
+EBO _;
+void f();
+};
+
+extern template void BS>::f();
+
+}}
+
+//--- b.h
+#pragma once
+
+#include "s.h"
+
+struct B {
+ void f() {
+N::S{}.f();
+ }
+};
+
+//--- a.h
+#pragma once
+
+#include "s.h"
+
+struct A {
+ void f(int) {}
+ void f(const N::S &) {}
+
+ void g();
+};
+
+//--- a.cpp
+#include "a.h"
+
+void A::g() { f(0); }
+
+// expected-no-diagnostics
>From e3aebf17e0ea72492564a3652518e6ad20f37330 Mon Sep 17 00:00:00 2001
From: Michael Park
Date: Thu, 6 Mar 2025 11:49:54 -0800
Subject: [PATCH 2/3] [clang][modules] Re-add the unit test from #121245.
---
clang/test/Modules/pr121245.cpp | 93 +
1 file changed, 93 insertions(+)
create mode 100644 clang/test/Modules/pr121245.cpp
diff --git a/clang/test/Modules/pr121245.cpp b/clang/test/Modules/pr121245.cpp
new file mode 100644
index 0..0e276ad0e435d
--- /dev/null
+++ b/clang/test/Modules/pr121245.cpp
@@ -0,0 +1,93 @@
+// If this test fails, it should be investigated under Debug builds.
+// Before the PR, this test was encountering an `llvm_unreachable()`.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
+// RUN: -fcxx-exceptions -o %t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-02.h \
+// RUN: -Wno-experimental-header-units -fcxx-exceptions \
+// RUN: -fmodule-file=%t/hu-01.pcm -o %t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-03.h \
+// RUN: -Wno-experimental-header-units -fcxx-exceptions \
+// RUN: -fmodule-file=%t/hu-01.pcm -o %t/hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-04.h \
+// RUN: -Wno-experimental-header-units -fcxx-exceptions \
+// RUN: -fmodule-file=%t/hu-01.pcm -o %t/hu-04.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-05.h \
+// RUN: -Wno-experimental-header-units -fcxx-exceptions \
+// RUN: -fmodule-file=%t/hu-03.pcm -fmodule-file=%t/hu-04.pcm \
+// RUN: -fmodule-file=%t/hu-01.pcm -o %t/hu-05.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj %t/main.cpp \
+// RUN: -Wno-experimental-header-units -fcxx-exceptions \
+// RUN: -fmodule-file=%t/hu-02.pcm -fmodule-file=%t/hu-05.pcm \
+// RUN: -fmodule-file=%t/hu-04.pcm -fmodule-file=%t/hu-03.pcm \
+// RUN: -fmodule-file=%t/hu-01.pcm
+
+//--- hu-01.h
+template
+struct A {
+ A() {}
+ ~A() {}
+};
+
+template
+struct EBO : T {
+ EBO() = default;
+};
+
+template
+struct HT : EBO> {};
+
+//--- hu-02.h
+import "hu-01.h";
+
+inline void f() {
+ HT();
+}
+
+//--- hu-03.h
+import "hu-01.h";
+
+struct C {
+ C();
+
+ HT _;
+};
+
+//--- hu-04.h
+import "hu-01.h";
+
+void g(HT = {});
+
+//--- hu-05.h
+import "hu-03.h";
+import "hu-04.h";
+import "hu-01.h";
+
+struct B {
+ virtual ~B() = default;
+
+ virtual void f() {
+HT();
+ }
+};
+
+//--- main.cpp
+import "hu-02.h";
+import "hu-05.h";
+import "hu-03.h";
+
+int main() {
+ f();
+ C();
+ B();
+}
>From 3134ad3f6fccfa3216ddcb750cdfa93f05ec Mon Sep 17 00:00:00 2001
From: Michael Park
Date: Thu, 6 Mar 2025 11:47:09 -0800
Subject: [PATCH 3/3] [clang][modules] Only update the generation number if the
update was not delayed.
---
clang/include/clang
[Lldb-commits] [clang] [lldb] [C++20][Modules] Do not update the generation number if the redecl chain completion was delayed. (PR #129982)
https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)
https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
