ilya-biryukov added a comment.
Many thanks! I didn't know about `split-file`, it's much nicer indeed!
Also incorporating feedback from @rsmith and removing the call to
`makeMergedDefinitionVisible`. Keeping just `setPrimaryMergedDecl` is enough
here.
Richard's reply from the email exchange:
I don't think that makeMergedDefinitionVisible is quite right; that's
intended to solve a different problem. Specifically, when we have a repeated
class definition in a single parse:
// in module X
class A { void f(); };
// in module Y
class A { void f(); };
Here, we first see and parse the class definition of A in module X. That then
becomes "the" definition of A, for the purposes of this compilation. Then when
we see another definition of the class, we choose to skip it because we already
have a definition, and Clang wants there to only be one definition of each
class. So we end up behaving as if we parsed this:
// in module X
class A { void f(); };
// in module Y
class A;
... but we still need to track that class A has a complete definition if only
Y is imported, so that A::f can be used for example. And that's what
makeMergedDefinitionVisible does: it says that A should be considered as having
a complete definition when Y is imported, even though the definition actually
lives in X.
For concepts I think the situation is different: we're not going to skip the
"body" of the concept and treat
// module X
template<typename T> concept bool C = true;
// module Y
template<typename T> concept bool C = true;
as if it were
// module X
template<typename T> concept bool C = true;
// module Y
template<typename T> concept bool C; // forward declaration?
... because there's no such thing as a forward declaration of a concept.
Instead, I think we should just be calling ASTContext::setPrimaryMergedDecl to
say that the C in module Y is a redeclaration of the C in module X, so that a
name lookup that finds both is not ambiguous. It shouldn't matter which one we
actually pick, because they are the same.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128921/new/
https://reviews.llvm.org/D128921
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
