[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

Michael Park via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 5 22:26:56 PST 2025


================
@@ -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:

Yeah, I agree. I'm kind of trying to figure out what the most recent pattern (😂) is for most recent decls.

Some options I've come across are:

1. `CompleteRedeclChain` if `getExternalSource()` is present. e.g. [SemaType.cpp](https://github.com/llvm/llvm-project/blob/release/20.x/clang/lib/Sema/SemaType.cpp#L9225-L9230) (current state of the PR)
```cpp
  // 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);
  }
```

2. "Dummy" invocation of `getMostRecentDecl()` if `getExternalSource()` is present. e.g. [DeclBase.cpp](https://github.com/llvm/llvm-project/blob/release/20.x/clang/lib/AST/DeclBase.cpp#L1884-L1889)
```cpp
  // If we have an external source, ensure that any later redeclarations of this
  // context have been loaded, since they may add names to the result of this
  // lookup (or add external visible storage).
  ExternalASTSource *Source = getParentASTContext().getExternalSource();
  if (Source)
    (void)cast<Decl>(this)->getMostRecentDecl();
```

3. Unconditional "dummy" invocation of `getMostRecentDecl()`. e.g. [CXXRecordDecl::dataPtr](https://github.com/llvm/llvm-project/blob/release/20.x/clang/include/clang/AST/DeclCXX.h#L457-L461) and [RecordLayoutBuilder.cpp](https://github.com/llvm/llvm-project/blob/release/20.x/clang/lib/AST/RecordLayoutBuilder.cpp#L3329-L3330)

I'm not quite sure I fully understood what you meant by:
> I feel better to do this in redecls.

We don't need to iterate through the `redecls()` here, and while I see dummy invocations of `getMostRecentDecl` as a way to trigger redecl completion, I don't see examples of `redecls` being called to force that to happen.

https://github.com/llvm/llvm-project/pull/129982


More information about the cfe-commits mailing list