rsmith added inline comments.
================
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:1181-1184
+ if (DD.Definition != NewDD.Definition) {
+ Reader.MergedDeclContexts.insert(
+ std::make_pair(NewDD.Definition, DD.Definition));
+ }
----------------
A couple of other things you should consider doing in this case:
* If there's an AST invariant that there is only one definition, think about
how to maintain that invariant. For other kinds of declaration, we would update
`NewDD` to make it act as a forward-declaration, but I'm not sure we can do
much about it here given that an `@interface` is never a forward-declaration.
Maybe for `@interface` there's nothing we can really do, and code dealing with
these declarations just needs to handle there possibly being more than one
definition?
* Merge the definition visibility of the new definition into the canonical
definition. This is necessary to ensure that in code for which the new
definition is imported and the canonical definition is not, we still treat the
canonical definition as being visible. (Eg, module A has the canonical
definition, module B has another definition, module C imports A but doesn't
re-export it, and C also imports B and re-exports it, and then you import C and
try to use the class.)
For the latter, `Reader.mergeDefinitionVisibility(DD.Definition,
NewDD.Definition)` should be sufficient, assuming that all code treats the
first definition as being the canonical one (for example, name lookups are
performed into the first definition).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110287/new/
https://reviews.llvm.org/D110287
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
