[C++20][Modules] Implementation of GMF decl elision.

[iains]

Summary

This implements the provisions of [module.global.frag] p3/4. This provides for discarding
declarations in the global module fragment if they are not reachable from any use within
the named module purview.

Assuming that the current TU has a global module fragment, we start out by creating decls
in that fragment marked as "ModuleDiscardable".

Then when in the module purview of the named module, whenever we encounter a use
or reference of a declaration, we check to see if that declaration (or any other declaration that
becomes indirectly reachable via it) is on the GMF. If so, then we mark that declaration as
reachable. We also check for cases that types in such declarations cause type decls in the
GMF to be reachable.

In the most general case, this can represent a significant amount of work - walking through
the bodies of functions to check for indirect reachability. To counter that as far as possible
we use two sets (one for decls, one for types) to ensure that each is only visited once.

[ChuanqiXu]

clang/lib/Sema/Sema.cpp

1115

I prefer to wrap this logic to a function to make it easier to read.

1116–1118

I prefer to cite the standard. And the original comment looks not so right (since we don't and couldn't remove a declaration in GMF due to it is not used by the body of the interface)

1123–1126

Prefer range based loop.

1127

We could sink the declaration of M to its definition.

1131

M == GlobalModuleFragment says that M is a global module fragment in the current TU explicitly. The original implementation doesn't check if M is in the current TU or not.

1135

Should we check for D->isUsed() simply? It looks more straight forward to me.

[ChuanqiXu]

clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp

31–32

This error message looks worse. I image I could hear users complaining this. (I doesn't say it is your fault since this is specified in standard and the standard cases are harder to understand). I want to know if this is consistent with GCC?

[rsmith]

clang/lib/Sema/Sema.cpp

1135

Does this work properly if the declaration is referenced within the header unit? I think we track whether we've seen any use of the declaration anywhere, not only if we've seen a use in the current translation unit, and we'll need a different mechanism here.

1139–1140

Please don't remove the declarations from the DeclContext; removeDecl is inefficient, not well-tested, and not really compatible with the Clang philosophy of AST fidelity. For example, this will be very problematic for tooling that wants to inspect the translation unit after compilation.

As an alternative that should also fix the issue with checking isUsed, how would you feel about this:

• Create a new ModuleOwnershipKind, say ReachableIfUsed, and set that as the ownership kind for the TU when parsing the global module fragment so it gets inherited into everything we parse in that region. This'll mean that NextInContextAndBits needs an extra bit for the ownership field, but I think Decl is 8-byte-aligned so that seems fine.
• When a declaration is being marked as used, check if its ownership kind is ReachableIfUsed and if so change it to ModulePrivate.

That should be enough to get the desired semantic effect, and would allow us to get the improved diagnostic experience that @ChuanqiXu asked about. As a potentially separate step, we can teach the ASTWriter code to (optionally) skip declarations that are ReachableIfUsed (and similarly for internal linkage declarations in module interfaces, function bodies for non-inline functions, and anything else we're allowed to omit).

[rsmith]

clang/lib/Sema/Sema.cpp

1135

s/header unit/global module fragment/

1139–1140

When a declaration is being marked as used, check if its ownership kind is ReachableIfUsed and if so change it to ModulePrivate.

We'd need something a little more subtle, such as checking whether the module ownership kind of the translation unit is no longer ReachableIfUsed, to detect whether we've already left the global module fragment at the point of use. Maybe there's somewhere in Sema that marks declarations as used that we can put this logic in instead?

[iains]

this is a re-write - please see additional comments on the revised direction.

[iains]

@ChuanqiXu, thanks for reviewing - but it seems I need to find the right direction before dealing with the details.

@rsmith - So here is a revised strategy - it is incomplete, and the code contains debug - so I am posting **only** for a review of the direction taken.

I first tried your suggested ReachableIfUsed directly - but that became somewhat tangled because, as you noted, used is not sufficient - we mark things as used outside of the module purview.

So.

I added ModuleUnreachable which is set to be the default module ownership for the GMF.

When we are in module purview and we mark a decl as used or referenced we reset that ownership to visible.
So Sema now has thin wrappers over setIsUsed/markUsed/setReferenced that then handle the case that the decl is marked as ModuleUnreachable.

Then comes the horrible bit; AFAICT we then have to walk the decl we just marked to determine if that makes other decls similarly visible - I've made a partial implementation of this (it just covers function decls - which is enough to run the example from the standard).
^^^ this is what I need review of direction-wise .. I cannot at present see a simpler way to do it - or even a reasonable way to cache used-ness dependencies in the GMF.

Any present, I've elected to stream the ModuleUnreachable state which is then used to ignore decls in lookup.
It seems harder than I'd expected to 'just not stream` those decls

• although that would be what I'd assume to be the motivation for this facility - reducing the size of PCMs for cases like:

module;
#include <something>
module Foo;
....

suggestions most welcome - if the direction is OK, then the markReachableGMFDecls function needs to be completed before reviewing the details.

clang/lib/Sema/Sema.cpp

1139–1140

When a declaration is being marked as used, check if its ownership kind is ReachableIfUsed and if so change it to ModulePrivate.

We'd need something a little more subtle, such as checking whether the module ownership kind of the translation unit is no longer ReachableIfUsed, to detect whether we've already left the global module fragment at the point of use. Maybe there's somewhere in Sema that marks declarations as used that we can put this logic in instead?

Yes, what I was doing was way too simplistic - please see the top-level comment now on the revised direction.

clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp

31–32

This error message looks worse. I image I could hear users complaining this. (I doesn't say it is your fault since this is specified in standard and the standard cases are harder to understand). I want to know if this is consistent with GCC?

As you say, the standard says "neither reachable nor visible"
if it's not reachable then. we would not have the information that it was from header foo.h so we cannot form the nicer diagnostic.

Perhaps we need to invent "reachable for diagnostics" ... which I'd rather not divert effort to right now.

[ChuanqiXu]

Given it touches ModuleOwnershipKind too and some codes looks similar to me (I haven't look into the it yet), I want to know if it is possible to make D113545 a parent revision of this one? I feel like reachability might be more important than this one. Since reachability helps developers to use something but this functionality prevent developers to use something.

[iains]

on the serialisation-front; maybe it would be better just to stream the 3 bits of the ownership and then we only need to check on deserialisation that the owning module exists for anything other than !owned.

[iains]

In D126694#3559818, @ChuanqiXu wrote:

Given it touches ModuleOwnershipKind too and some codes looks similar to me (I haven't look into the it yet), I want to know if it is possible to make D113545 a parent revision of this one? I feel like reachability might be more important than this one. Since reachability helps developers to use something but this functionality prevent developers to use something.

I will look at this - it is indeed a problem that currently we do not differentiate reachability from visibility - which means either some work-around, or making some thing artificially visible,
this patch would address your

/// FIXME: Implement discarding declarations actually in global module
/// fragment. See [module.global.frag]p3,4 for details.

[iains]

reworked to use D113545 as a parent revision

which resolves the issue of needing to make module ownership 'visible' to satisfy lookup.

[iains]

so standard 10.4 ex 2 gives the expected output with this patch stack
note that there's a query to core about whether that example is fully correct (but that does not affect the skeleton of these implements - only the detail of what is included / excluded in the markReachableGMFDecls() evaluation.

[ChuanqiXu]

In D126694#3561257, @iains wrote:

so standard 10.4 ex 2 gives the expected output with this patch stack

Cool, and I think we should add that one as a test if it works. Tests are always good.

---

BTW, I guess we could remove dependencies with D126959 , D126189, D126691. If I understand things correctly, there shouldn't be dependencies between them.

clang/include/clang/AST/DeclBase.h

655

Maybe we need to check if the owning module is global module or not.

clang/include/clang/Sema/Sema.h

2303

I feel better if we would check if D lives in GMF. (We need to insert a check in isDiscardedInGlobalModuleFragment)

clang/lib/AST/TextNodeDumper.cpp

1652

It may be better to keep the consistent style.

1663

ditto

1671

ditto

1700

ditto

1780

ditto

1799

ditto

clang/lib/Sema/SemaModule.cpp

269

Given Interface is only assigned in the case of ModuleDeclKind::Implementation, it looks possible to sink the declaration to the place it get assigned. In this way, we could avoid the confusion here.

357

This comes from https://reviews.llvm.org/D126959. I feel like odd to return the import declaration that time. I feel like it is better to return the module declaration itself instead of the imported one. Let's return nullptr now and fix it in the future.

359–360

So that we could hoist the codes.

977–978

991

It looks better to add assumption as an assertion .

992–993

1009–1012

1016–1017

clang prefer shorter indentation.

1022

Remove dump

clang/lib/Serialization/ASTReaderDecl.cpp

633–637

Maybe we could make a new bool variable like ModulePrivate to keep the style consistent. So that we could write:

647–650

clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp

31–32

Maybe we could make a new diagnostic message.

[iains]

rebased and removed dependency on p1874 initializer patch.

Some tidying - added 104. ex2 testcase.

[iains]

again, thanks for review - but please do not spend any effort on style points yet - the debug and dump stuff is intentionally present this is "for comment on the approach"

i.e. what is important is to establish that this is a reasonable approach.

As of now we have the following fails - which have to be analysed (of course, markReachableGMFDecls is expected to be incomplete at this point.

FAIL: Clang :: CXX/basic/basic.lookup/basic.lookup.argdep/p4-friend-in-reachable-class.cpp (3992 of 15859)
FAIL: Clang :: Modules/derived_class.cpp (6764 of 15859)
FAIL: Clang :: Modules/explicitly-specialized-template.cpp (6831 of 15859)
FAIL: Clang :: Modules/template-function-specialization.cpp (7060 of 15859)
FAIL: Clang :: Modules/template_default_argument.cpp (7207 of 15859)

clang/include/clang/AST/DeclBase.h

655

The only place that the ownership is ModuleUnreachable is in the GMF - so that we do not need to do an additional test.

clang/include/clang/Sema/Sema.h

2303

If the consensus is to add an extra test, OK.

However, as above, specifically to avoid making more and more tests in code that is executed very frequently - as the design currently stands, the only place that ModuleUnreachable is set is in the GMF.

clang/lib/AST/TextNodeDumper.cpp

1652

I don't think that is a matter of style __module_private__ is a keyword used elsewhere?

If you look though the file you will see mostly that the printed output does not prepend or append underscores.

BTW, similar changes are probably needed in other node printers, this was done early to add debug.

clang/lib/Sema/SemaModule.cpp

269

this code has been removed (it belongs in the D126959 patch).

(not relevant to the patch, but for the record), for the functionality to be correct - we must ensure that the interface module is registered first - so that the module names table contains the pointer to that

359–360

this has been reorganised

991

what is D here?
markReachableGMFDecls is only called in the case that Orig is not discarded (we are marking decls as reachable because they are used in the interface..

[iains]

clang/lib/Serialization/ASTReaderDecl.cpp

633–637

yes, perhaps we could do that - I am wondering if that code can all be factored into the switch.

clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp

31–32

I do not think it is a matter of a diagnostic message.

If we omit the decl from the BMI (which we are permitted to do if it is ModuleUnreachable), then it cannot be found and therefore the information on which header it came from would not be available.

[iains]

the first failure is like this:

x-h.h:
struct A {
  friend int operator+(const A &lhs, const A &rhs) {
    return 0;
  }
};

X.cpp:
module;
#include "x-h.h"
export module X;

export using ::A;

This does not mark *anything* in the GMF (x-h.h) as 'used', and so everything there is unreachable (and hence the fails).
I.e export using ::A; neither marks A as used or referenced.
I am not sure if we are supposed to special-case this - since https://eel.is/c++draft/module#global.frag-3.6 explicitly says "In this determination, it is unspecified .. whether using-declaration, ... is replaced by the declarations they name prior to this determination,
so .. not about how to proceed with this one at present;
edit: but it seems most reasonable to make it behave as if A was itself exported.

[ChuanqiXu]

In D126694#3564629, @iains wrote:

the first failure is like this:

x-h.h:
struct A {
  friend int operator+(const A &lhs, const A &rhs) {
    return 0;
  }
};

X.cpp:
module;
#include "x-h.h"
export module X;

export using ::A;

This does not mark *anything* in the GMF (x-h.h) as 'used', and so everything there is unreachable (and hence the fails).
I.e export using ::A; neither marks A as used or referenced.
I am not sure if we are supposed to special-case this - since https://eel.is/c++draft/module#global.frag-3.6 explicitly says "In this determination, it is unspecified .. whether using-declaration, ... is replaced by the declarations they name prior to this determination,
so .. not about how to proceed with this one at present;
edit: but it seems most reasonable to make it behave as if A was itself exported.

I highly recommend we should mark A here. Maybe we need other interfaces than markDeclUsed and setDeclReferenced. If we don't support this, we couldn't use modules like https://github.com/alibaba/async_simple/blob/CXX20Modules/third_party_module/asio/asio.cppm. This manner is important to use C++20 modules before the build system is ready. Also, I think it is an important tool to implement C++23's std modules. So we should support it.

clang/include/clang/AST/DeclBase.h

240

Would you like to use the term 'ModuleDiscarded'? My point is that not all unreachable declaration in modules are in GMF. And the term discard is used in standard to describe it. So it looks better.

655

It is more robust and clear to add the additional check to me. Since the constraint now lives in the comment only. If anyone goes to change it, the codes wouldn't complain.

clang/include/clang/Sema/Sema.h

2303

Yeah, my opinion is the same as above. Although it is the design, it is more semantically clear and robust to add a additional check. I am just afraid it would confuse and block other readers or contributors.

clang/lib/AST/TextNodeDumper.cpp

1652

Oh, I found __module_private__ is a keyword in clang modules. I didn't recognize it. Even in this case, I still prefer to keep the style consistently. I think users would be more comfortable to read consistent symbols. Also I think it is acceptable to keep ModuleUnreachable since it doesn't matter a lot to me.

clang/lib/Sema/SemaModule.cpp

991

Oh, D should be Orig, my bad. Yeah, I found markReachableGMFDecls is only called when Orig is not discarded. So I suggest to add the assertion to make it explicit and clear and it could avoid misuse in the future.

clang/lib/Serialization/ASTReaderDecl.cpp

633–637

It looks possible.

clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp

31–32

Your word makes sense.

clang/test/Modules/cxx20-10-4-ex2.cpp

44 ↗

(On Diff #434834)

There is invisible symbol.

[iains]

In D126694#3565417, @ChuanqiXu wrote:

In D126694#3564629, @iains wrote:

the first failure is like this:

x-h.h:
struct A {
  friend int operator+(const A &lhs, const A &rhs) {
    return 0;
  }
};

X.cpp:
module;
#include "x-h.h"
export module X;

export using ::A;

This does not mark *anything* in the GMF (x-h.h) as 'used', and so everything there is unreachable (and hence the fails).
I.e export using ::A; neither marks A as used or referenced.
I am not sure if we are supposed to special-case this - since https://eel.is/c++draft/module#global.frag-3.6 explicitly says "In this determination, it is unspecified .. whether using-declaration, ... is replaced by the declarations they name prior to this determination,
so .. not about how to proceed with this one at present;
edit: but it seems most reasonable to make it behave as if A was itself exported.

I highly recommend we should mark A here. Maybe we need other interfaces than markDeclUsed and setDeclReferenced. If we don't support this, we couldn't use modules like https://github.com/alibaba/async_simple/blob/CXX20Modules/third_party_module/asio/asio.cppm. This manner is important to use C++20 modules before the build system is ready. Also, I think it is an important tool to implement C++23's std modules. So we should support it.

Actually, after thinking some more, what seems to be wrong here is that we should be making the exported item "VisibleIfImported" .. which is not being done - I guess this was a bug already and it has been exposed by the recent changes in the module ownership processing. I will next take a look at that (and the other comments).

[iains]

this is a rework of the implementation.

it now passes all test-cases except one - which is one of the cases added
by D113545; that test-case now seems to produce multiple error messages for
each of the cases that it is required to (i.e. the lines that should error
do produce an error - but they also produce a number of similar or duplicated
errors).

[iains]

@ChuanqiXu - I changed the module ownership name to "ModuleDiscardable" - because, while we are permitted to discard these, we might choose not to (to give your better diagnostics) - but we still need to treat them as non-reachable and non-visible. Please could you examine what is happening with Modules/explicitly-specialized-template.cpp where there a multiple error messages for each (intentionally) failing line .. add the test from the std.

@rsmith
this seems like an awful lot of work that has to be done for every decl we mark used in the module purview - we cannot even short-cut ones that are not in the GMF, since the provisions of [module.global.frag] p3.5 seem to allow this to happen for indirect reaching. I wonder if I misread the std.
I am also wondering what is supposed to happen when an interface makes a type reachable, but that type is not visible to the implementation .. it seems to mean that interfaces would have to add using declarations for each such type.

clang/include/clang/AST/DeclBase.h

240

ModuleDiscardable Is better, it says we have permission to discard it (so that it cannot be used or reached) but allows for the case we elect to keep them around for better diagnostics. You might want to consider renaming the wrapper function similarly?

655

I am very concerned about the amount of work this (GMF decl elision) adds, so would like to minimise this - what I have done is to add an assert at the point we might make a change to the ownership that tests to see the decl is in the fragment we expect.

clang/lib/Sema/SemaModule.cpp

1022

of course, as noted, the patch here is for comment on approach.

[ChuanqiXu]

In D126694#3576602, @iains wrote:

@ChuanqiXu - I changed the module ownership name to "ModuleDiscardable" - because, while we are permitted to discard these, we might choose not to (to give your better diagnostics) - but we still need to treat them as non-reachable and non-visible. Please could you examine what is happening with Modules/explicitly-specialized-template.cpp where there a multiple error messages for each (intentionally) failing line .. add the test from the std.

I've looked into this. The multiple duplicated diagnostic message is a legacy problem. I tried to fix it but it is hard (change it would break many other tests). To filter out the redundant duplicated diagnostic message, you could use '+1' suffix after expected-error. For example: https://github.com/llvm/llvm-project/blob/16ca490f450ea3ceaeda92addd8546967af4b2e1/clang/test/Modules/cxx-templates.cpp#L215-L232 BTW, after I filtered out the duplicated diagnostic message, I find the complains the use of foo<int> in the module purview, which is not right and should be a bug of this revision.

@rsmith
this seems like an awful lot of work that has to be done for every decl we mark used in the module purview - we cannot even short-cut ones that are not in the GMF, since the provisions of [module.global.frag] p3.5 seem to allow this to happen for indirect reaching. I wonder if I misread the std.

Yeah, and I am not sure what is better idea here. I tried to implement GMF decl elision before. My first idea is similar to your first revision. But I tried to implement indirect discarding that time. So I chose to traverse the decls in GMF until a fixed point when we handle the end of the TU. It looks bad clearly (the time complexity is not satisfying). So I didn't post it up.

I am also wondering what is supposed to happen when an interface makes a type reachable, but that type is not visible to the implementation .. it seems to mean that interfaces would have to add using declarations for each such type.

@rsmith should be the best to answer here. But I am trying to answer it. If 'the implementation' means the implementation unit, I think the type is naturally visible to the implementation unit. We could find the example in: module.interface-example5 .

clang/include/clang/AST/DeclBase.h

240

From my reading, it looks like the std says "we should discard things in following cases... although there are some cases we could make decision by ourself". So it looks like ModuleDiscarded is better than ModuleDiscardable to me.

655

How about something like:

assert (getModuleOwnershipKind() != ModuleOwnershipKind::ModuleDiscardable || getOwningModule()->isGlobalModule);
return getModuleOwnershipKind() == ModuleOwnershipKind::ModuleDiscardable;

So that we could get the clear semantics and the performance.

[iains]

rebased, fixed the fail for explicitly-specialized-template.cpp.

[iains]

In D126694#3576853, @ChuanqiXu wrote:

In D126694#3576602, @iains wrote:

@ChuanqiXu - I changed the module ownership name to "ModuleDiscardable" - because, while we are permitted to discard these, we might choose not to (to give your better diagnostics) - but we still need to treat them as non-reachable and non-visible. Please could you examine what is happening with Modules/explicitly-specialized-template.cpp where there a multiple error messages for each (intentionally) failing line .. add the test from the std.

I've looked into this. The multiple duplicated diagnostic message is a legacy problem. I tried to fix it but it is hard (change it would break many other tests). To filter out the redundant duplicated diagnostic message, you could use '+1' suffix after expected-error. For example: https://github.com/llvm/llvm-project/blob/16ca490f450ea3ceaeda92addd8546967af4b2e1/clang/test/Modules/cxx-templates.cpp#L215-L232 BTW, after I filtered out the duplicated diagnostic message,

This was not the problem in this particular case - but (for reference) is there an issue for the duplicated diagnostics? - that seems not very user-friendly, especially since C++ emits a lot of long diagnostics already.

I find the complains the use of foo<int> in the module purview, which is not right and should be a bug of this revision.

Yes, fixed with the latest upload; the provisions of [module.global.frag] p3 are quite complex, it's easy to miss one (and now the explicitly-specialized-template.cpp tase passes too)

@rsmith
this seems like an awful lot of work that has to be done for every decl we mark used in the module purview - we cannot even short-cut ones that are not in the GMF, since the provisions of [module.global.frag] p3.5 seem to allow this to happen for indirect reaching. I wonder if I misread the std.

Yeah, and I am not sure what is better idea here. I tried to implement GMF decl elision before. My first idea is similar to your first revision. But I tried to implement indirect discarding that time. So I chose to traverse the decls in GMF until a fixed point when we handle the end of the TU. It looks bad clearly (the time complexity is not satisfying). So I didn't post it up.

I am also wondering what is supposed to happen when an interface makes a type reachable, but that type is not visible to the implementation .. it seems to mean that interfaces would have to add using declarations for each such type.

@rsmith should be the best to answer here. But I am trying to answer it. If 'the implementation' means the implementation unit, I think the type is naturally visible to the implementation unit. We could find the example in: module.interface-example5 .

That is not the problem I am describing - which more like:

type.h:
typedef int MyInt;

mod-interface.cpp:
module;
#include "type.h"
export module A;

MyInt foo (); // makes MyInt reachable, but not visible.

mod-imp.cpp:
module A;

MyInt foo () { return 42; } // will not compile because the type declaration is not visible.

I am told that this is the intention .. it just seems a wee bit odd.

clang/include/clang/AST/DeclBase.h

240

I think we should observe the "as if" rule here, and say that the implementation would behave "as if" the decls were removed (but we choose to leave them, at least for now, because that improves the diagnostic you mentioned before).

The name should be helpful to programmers - if we make it "Discarded" then it will surely be confusing to a programmer to find that the decls are still present - "Discardable" says the right thing (that you should not use them because at some time they could be removed).

655

let's finalise this once we have the rest of the patch in reasonable shape and the approach is agreed to be an acceptable way forward.

[ChuanqiXu]

In D126694#3577815, @iains wrote:

In D126694#3576853, @ChuanqiXu wrote:

In D126694#3576602, @iains wrote:

@ChuanqiXu - I changed the module ownership name to "ModuleDiscardable" - because, while we are permitted to discard these, we might choose not to (to give your better diagnostics) - but we still need to treat them as non-reachable and non-visible. Please could you examine what is happening with Modules/explicitly-specialized-template.cpp where there a multiple error messages for each (intentionally) failing line .. add the test from the std.

I've looked into this. The multiple duplicated diagnostic message is a legacy problem. I tried to fix it but it is hard (change it would break many other tests). To filter out the redundant duplicated diagnostic message, you could use '+1' suffix after expected-error. For example: https://github.com/llvm/llvm-project/blob/16ca490f450ea3ceaeda92addd8546967af4b2e1/clang/test/Modules/cxx-templates.cpp#L215-L232 BTW, after I filtered out the duplicated diagnostic message,

This was not the problem in this particular case - but (for reference) is there an issue for the duplicated diagnostics? - that seems not very user-friendly, especially since C++ emits a lot of long diagnostics already.

I failed to find one and I filed one at: https://github.com/llvm/llvm-project/issues/56024. Hope this helps.

@rsmith
this seems like an awful lot of work that has to be done for every decl we mark used in the module purview - we cannot even short-cut ones that are not in the GMF, since the provisions of [module.global.frag] p3.5 seem to allow this to happen for indirect reaching. I wonder if I misread the std.

Yeah, and I am not sure what is better idea here. I tried to implement GMF decl elision before. My first idea is similar to your first revision. But I tried to implement indirect discarding that time. So I chose to traverse the decls in GMF until a fixed point when we handle the end of the TU. It looks bad clearly (the time complexity is not satisfying). So I didn't post it up.

I am also wondering what is supposed to happen when an interface makes a type reachable, but that type is not visible to the implementation .. it seems to mean that interfaces would have to add using declarations for each such type.

@rsmith should be the best to answer here. But I am trying to answer it. If 'the implementation' means the implementation unit, I think the type is naturally visible to the implementation unit. We could find the example in: module.interface-example5 .

That is not the problem I am describing - which more like:

type.h:
typedef int MyInt;

mod-interface.cpp:
module;
#include "type.h"
export module A;

MyInt foo (); // makes MyInt reachable, but not visible.

mod-imp.cpp:
module A;

MyInt foo () { return 42; } // will not compile because the type declaration is not visible.

I am told that this is the intention .. it just seems a wee bit odd.

Oh, it makes sense now. We could include "type.h" in the implementation unit so it wouldn't matter I think.

[iains]

reabsed onto latest version of D113545.

[ChuanqiXu]

clang/include/clang/AST/DeclBase.h

632

According to the opinion from @rsmith, the discarded declaration is private too.

[iains]

rebased after D113545 was landed and removed that as a parent.

clang/include/clang/AST/DeclBase.h

632

I guess you mean >= ... however Discardable is a stronger constraint than Private

If a decl remains marked Discardable (after the processing to determine reachable ones) that means it is both unreachable and invisible.
So it must not participate in any processing (with the one exception of diagnostic output). I would be concerned that the change you suggest above could cause a Discardable decl to be considered in merging or lookup and we would then need (maybe a lot) of logic like:

if (D->isModulePrivate() && !D->isModuleDiscardable())
   ...

I will take a look on the next iteration.

[rsmith]

clang/include/clang/Sema/Sema.h

2298–2299

These names seem too general to live directly in Sema.

2301

clang/lib/Sema/SemaModule.cpp

935–939

Do we need the special MakeVisible handling here? I would have thought that it would be sufficient for Child itself to be visible. Making the target of a using declaration visible seems like it would do the wrong thing for a case like:

module;
int f();
export module M;
namespace N { export using ::f; }

import M;
int a = f(); // should be an error, ::f should not be visible
int b = N::f(); // should be OK, UsingShadowDecl is visible

I wonder if we need any special handling here at all -- if the GMF decls are referenced by something in an export block, then I'd have expected they'll already be marked as used and that marking would have marked them as reachable.

1040–1052

Instead of storing a SeenDecls set and checking it here, is it feasible to check whether we're transitioning this declaration from discardable to retained, and only doing the work below if so?

[ChuanqiXu]

BTW, after I applied the patch, the compiler crashes at https://github.com/ChuanqiXu9/stdmodules. I would try to add more tests about C++20 Modules.

[iains]

In D126694#3629094, @ChuanqiXu wrote:

BTW, after I applied the patch, the compiler crashes at https://github.com/ChuanqiXu9/stdmodules.

That link points to a project - is there (say) a gist of the crash information?

I would try to add more tests about C++20 Modules.

of course, more tests can be useful, but it would be better to try to be specific - it does not crash for any of the tests in the clang test suite.

[ChuanqiXu]

In D126694#3629251, @iains wrote:

In D126694#3629094, @ChuanqiXu wrote:

BTW, after I applied the patch, the compiler crashes at https://github.com/ChuanqiXu9/stdmodules.

That link points to a project - is there (say) a gist of the crash information?

Here is the crash log:

unhandled type: 0x7cda9e0 LValueReferenceType 0x7cda9e0 'const struct std::_PairT &'
`-QualType 0x7cd85f1 'const struct std::_PairT' const
  `-RecordType 0x7cd85f0 'struct std::_PairT'
    `-CXXRecord 0x7cd8540 '_PairT'
unhandled type: 0x7cdabb0 RValueReferenceType 0x7cdabb0 'struct std::_PairT &&'
`-RecordType 0x7cd85f0 'struct std::_PairT'
  `-CXXRecord 0x7cd8540 '_PairT'
unhandled type: 0x80e0400 LValueReferenceType 0x80e0400 'struct std::_PairT &'
`-RecordType 0x7cd85f0 'struct std::_PairT'
  `-CXXRecord 0x7cd8540 '_PairT'
unhandled type: 0x7c0e1a0 TemplateTypeParmType 0x7c0e1a0 '_T1' dependent depth 0 index 0
`-TemplateTypeParm 0x7c0e148 '_T1'
unhandled type: 0x7c0e220 TemplateTypeParmType 0x7c0e220 '_T2' dependent depth 0 index 1
`-TemplateTypeParm 0x7c0e1d0 '_T2'
unhandled type: 0x7c0e6a0 LValueReferenceType 0x7c0e6a0 'const pair<_T1, _T2> &' dependent
`-QualType 0x7ace031 'const pair<_T1, _T2>' const
  `-InjectedClassNameType 0x7ace030 'pair<_T1, _T2>' dependent
    `-CXXRecord 0x7c0e290 'pair'
unhandled type: 0x7c0e8e0 RValueReferenceType 0x7c0e8e0 'pair<_T1, _T2> &&' dependent
`-InjectedClassNameType 0x7ace030 'pair<_T1, _T2>' dependent
  `-CXXRecord 0x7c0e290 'pair'
clang++: /home/chuanqi.xcq/workspace.xuchuanqi/llvm-project-for-work/clang/lib/Sema/SemaModule.cpp:1084: void clang::Sema::markGMFDeclsReachableFrom(clang::Decl*, bool): Assertion `T->isRecordType() && "base not a record?"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: clang++ -std=c++20 --precompile -fprebuilt-module-path=. coroutine.cppm -o std-coroutine.pcm -isystem ../build_libcxx/include/c++/v1 -I../build_libcxx/include/x86_64-unknown-linux-gnu/c++/v1 -nostdinc++
1.	<eof> parser at end of file
2.	../build_libcxx/include/c++/v1/__functional/hash.h:308:12: instantiating function definition 'std::__scalar_hash<std::_PairT, 2>::operator()'
3.	../build_libcxx/include/c++/v1/__functional/hash.h:93:18: instantiating function definition 'std::__murmur2_or_cityhash<unsigned long, 64>::operator()'
 #0 0x0000000001f60f90 PrintStackTraceSignalHandler(void*) Signals.cpp:0:0
 #1 0x0000000001f5ede4 llvm::sys::CleanupOnSignal(unsigned long) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x1f5ede4)
 #2 0x0000000001eaa468 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #3 0x00007ffff7fb29d0 __restore_rt sigaction.c:0:0
 #4 0x00007ffff7675f35 raise (/lib64/libc.so.6+0x3bf35)
 #5 0x00007ffff765f8d7 abort (/lib64/libc.so.6+0x258d7)
 #6 0x00007ffff765f7a7 _nl_load_domain.cold loadmsgcat.c:0:0
 #7 0x00007ffff766e536 (/lib64/libc.so.6+0x34536)
 #8 0x0000000004696390 clang::Sema::markGMFDeclsReachableFrom(clang::Decl*, bool) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4696390)
 #9 0x0000000004695d87 clang::Sema::markGMFDeclsReachableFrom(clang::Decl*, bool) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4695d87)
#10 0x0000000004696559 clang::Sema::findGMFReachableDeclsForType(clang::Type const*) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4696559)
#11 0x000000000469689b clang::Sema::findGMFReachableDeclExprs(clang::Stmt*) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x469689b)
#12 0x00000000046967bd clang::Sema::findGMFReachableDeclExprs(clang::Stmt*) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x46967bd)
#13 0x00000000046967bd clang::Sema::findGMFReachableDeclExprs(clang::Stmt*) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x46967bd)
#14 0x00000000046967bd clang::Sema::findGMFReachableDeclExprs(clang::Stmt*) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x46967bd)
#15 0x00000000046967bd clang::Sema::findGMFReachableDeclExprs(clang::Stmt*) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x46967bd)
#16 0x00000000046958c2 clang::Sema::markGMFDeclsReachableFrom(clang::Decl*, bool) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x46958c2)
#17 0x0000000004695749 clang::Sema::markGMFDeclsReachableFrom(clang::Decl*, bool) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4695749)
#18 0x0000000004695e75 clang::Sema::markGMFDeclsReachableFrom(clang::Decl*, bool) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4695e75)
#19 0x0000000004695e75 clang::Sema::markGMFDeclsReachableFrom(clang::Decl*, bool) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4695e75)
#20 0x0000000004695d87 clang::Sema::markGMFDeclsReachableFrom(clang::Decl*, bool) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4695d87)
#21 0x0000000004695b71 clang::Sema::markGMFDeclsReachableFrom(clang::Decl*, bool) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4695b71)
#22 0x000000000469653b clang::Sema::findGMFReachableDeclsForType(clang::Type const*) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x469653b)
#23 0x0000000004696114 clang::Sema::markGMFDeclsReachableFrom(clang::Decl*, bool) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4696114)
#24 0x0000000004a15993 clang::Sema::BuildVariableInstantiation(clang::VarDecl*, clang::VarDecl*, clang::MultiLevelTemplateArgumentList const&, llvm::SmallVector<clang::Sema::LateInstantiatedAttribute, 16u>*, clang::DeclContext*, clang::LocalInstantiationScope*, bool, clang::VarTemplateSpecializationDecl*) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4a15993)
#25 0x0000000004a1f3be clang::TemplateDeclInstantiator::VisitVarDecl(clang::VarDecl*, bool, llvm::ArrayRef<clang::BindingDecl*>*) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4a1f3be)
#26 0x0000000004a241f4 void llvm::function_ref<void ()>::callback_fn<clang::Sema::SubstDecl(clang::Decl*, clang::DeclContext*, clang::MultiLevelTemplateArgumentList const&)::'lambda'()>(long) SemaTemplateInstantiateDecl.cpp:0:0
#27 0x000000000411d7a1 clang::Sema::runWithSufficientStackSpace(clang::SourceLocation, llvm::function_ref<void ()>) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x411d7a1)
#28 0x00000000049d7042 clang::Sema::SubstDecl(clang::Decl*, clang::DeclContext*, clang::MultiLevelTemplateArgumentList const&) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x49d7042)
#29 0x000000000498fb15 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformDeclStmt(clang::DeclStmt*) SemaTemplateInstantiate.cpp:0:0
#30 0x00000000049c9479 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCompoundStmt(clang::CompoundStmt*, bool) SemaTemplateInstantiate.cpp:0:0
#31 0x00000000049cdfa5 clang::Sema::SubstStmt(clang::Stmt*, clang::MultiLevelTemplateArgumentList const&) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x49cdfa5)
#32 0x0000000004a197af clang::Sema::InstantiateFunctionDefinition(clang::SourceLocation, clang::FunctionDecl*, bool, bool, bool) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4a197af)
#33 0x0000000004a17c0f clang::Sema::PerformPendingInstantiations(bool) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4a17c0f)
#34 0x0000000004a19a3c clang::Sema::InstantiateFunctionDefinition(clang::SourceLocation, clang::FunctionDecl*, bool, bool, bool) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4a19a3c)
#35 0x0000000004a17c0f clang::Sema::PerformPendingInstantiations(bool) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x4a17c0f)
#36 0x000000000413b45a clang::Sema::ActOnEndOfTranslationUnitFragment(clang::Sema::TUFragmentKind) (.part.0) Sema.cpp:0:0
#37 0x000000000413bba6 clang::Sema::ActOnEndOfTranslationUnit() (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x413bba6)
#38 0x0000000003ff6448 clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&, clang::Sema::ModuleImportState&) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x3ff6448)
#39 0x0000000003feae7a clang::ParseAST(clang::Sema&, bool, bool) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x3feae7a)
#40 0x0000000002a66409 clang::FrontendAction::Execute() (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x2a66409)
#41 0x00000000029f2ec6 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x29f2ec6)
#42 0x0000000002b40433 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x2b40433)
#43 0x0000000000a0edca cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0xa0edca)
#44 0x0000000000a08298 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) driver.cpp:0:0
#45 0x000000000286cd75 void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const::'lambda'()>(long) Job.cpp:0:0
#46 0x0000000001eaa5d4 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x1eaa5d4)
#47 0x000000000286d46f clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const (.part.0) Job.cpp:0:0
#48 0x0000000002838d52 clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x2838d52)
#49 0x000000000283983d clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&, bool) const (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x283983d)
#50 0x00000000028430dc clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0x28430dc)
#51 0x0000000000a0d343 clang_main(int, char**) (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0xa0d343)
#52 0x00007ffff7661193 __libc_start_main (/lib64/libc.so.6+0x27193)
#53 0x0000000000a07ebe _start (/disk2/workspace.xuchuanqi/llvm-project-for-work/build/bin/clang-14+0xa07ebe)
clang-14: error: clang frontend command failed with exit code 134 (use -v to see invocation)
clang version 15.0.0 ([e-mail]:llvm/llvm-project.git 487d8ba3f33b127a7996ab9d3ba9c056e5e5b8c2)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /home/chuanqi.xcq/workspace.xuchuanqi/llvm-project-for-work/build/bin
clang-14: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang-14: note: diagnostic msg: /tmp/coroutine-73dc76.cppm
clang-14: note: diagnostic msg: /tmp/coroutine-73dc76.sh
clang-14: note: diagnostic msg: 

********************

I would try to add more tests about C++20 Modules.

of course, more tests can be useful, but it would be better to try to be specific - it does not crash for any of the tests in the clang test suite.

Yeah, small test is helpful for debugging and developing. But large examples are an important part for testing. Although there are many tests in clang test suite, we often find breaking changes in new patches. So it would be better to have both.

[iains]

rebased, addressed review comments, added another test.

[iains]

@rsmith, @ChuanqiXu apologies for the multiple revisions, this has turned out to be much more involved than I imagined from the standard's text.

In D126694#3629254, @ChuanqiXu wrote:

In D126694#3629251, @iains wrote:

In D126694#3629094, @ChuanqiXu wrote:

BTW, after I applied the patch, the compiler crashes at https://github.com/ChuanqiXu9/stdmodules.

That link points to a project - is there (say) a gist of the crash information?

Here is the crash log:

this code now compiles without error,

clang/include/clang/AST/DeclBase.h

632

I did try this and there are a number of regressions - when I looked into these there is some interaction with the changes made in D113545, so I think we should make these changes in a follow-one patch to avoid having two purposes in this one.

clang/include/clang/Sema/Sema.h

2298–2299

(revised we only need to track the type pointers)

On the basis that the name is poor, for its intended purpose, I revised.

2303

this has now been revised and a check is applied before any change is made to discardable decls.

clang/lib/AST/TextNodeDumper.cpp

1652

OK we now have more fine-grained output for the module ownership in the dumps which allows specific tests to be constructed. At present, the naming is as per decl.h (with the single exception of module_private which has a user-facing representation).

clang/lib/Sema/Sema.cpp

1115

I think the comment refers to an older version of the changes.

1116–1118

this is now implemented differently,

1123–1126

this is now implemented differently

1131

this is now implemented differently

1135

this is now implemented differently

clang/lib/Sema/SemaModule.cpp

935–939

Do we need the special MakeVisible handling here? I would have thought that it would be sufficient for Child itself to be visible. Making the target of a using declaration visible seems like it would do the wrong thing for a case like:

module;
int f();
export module M;
namespace N { export using ::f; }

import M;
int a = f(); // should be an error, ::f should not be visible
int b = N::f(); // should be OK, UsingShadowDecl is visible

fixed, and added a test case to cover this.

I wonder if we need any special handling here at all -- if the GMF decls are referenced by something in an export block, then I'd have expected they'll already be marked as used and that marking would have marked them as reachable.

Indeed. this was over cautious, However, the one case that we have to cover is the target of a using decl - those are neither marked used nor referenced.

991

in the revised code, we have an assert.

1040–1052

I had that as my original implementation, which I revised to do the same kind of thing as the type pointers. However, the check for isModuleDiscardable is cheaper, indeed.

(JFTR, I also tried an implementation with a decl visitor, but it did not seem to be any less complex or really any shorter code-wise)

I cannot see how we can avoid tracking the types.

clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp

31–32

in the current implementation, we mark the decls but do not yet actually remove them - so that the better diagnostic should still be available.

[ChuanqiXu]

In D126694#3635207, @iains wrote:

@rsmith, @ChuanqiXu apologies for the multiple revisions, this has turned out to be much more involved than I imagined from the standard's text.

In D126694#3629254, @ChuanqiXu wrote:

In D126694#3629251, @iains wrote:

In D126694#3629094, @ChuanqiXu wrote:

BTW, after I applied the patch, the compiler crashes at https://github.com/ChuanqiXu9/stdmodules.

That link points to a project - is there (say) a gist of the crash information?

Here is the crash log:

this code now compiles without error,

Thanks for looking into it!

My personal plan for this revision is to review the details after we add more large tests (at least we have a more complete std modules implementation and I am trying for it. But I find another bug now). So we might need to wait for a while for this patch. How do you think about it?

[iains]

In D126694#3637690, @ChuanqiXu wrote:

In D126694#3635207, @iains wrote:

@rsmith, @ChuanqiXu apologies for the multiple revisions, this has turned out to be much more involved than I imagined from the standard's text.

In D126694#3629254, @ChuanqiXu wrote:

In D126694#3629251, @iains wrote:

In D126694#3629094, @ChuanqiXu wrote:

BTW, after I applied the patch, the compiler crashes at https://github.com/ChuanqiXu9/stdmodules.

That link points to a project - is there (say) a gist of the crash information?

Here is the crash log:

this code now compiles without error,

Thanks for looking into it!

My personal plan for this revision is to review the details after we add more large tests (at least we have a more complete std modules implementation and I am trying for it. But I find another bug now). So we might need to wait for a while for this patch. How do you think about it?

Well, the difficulty there is that "add more large tests" is not a very specific objective.
I will be first to say that we can tell that the implementation here is necessary, but we cannot tell if it is sufficient - however, IMO we need to find a more definite way to make progress.

[ChuanqiXu]

In D126694#3638069, @iains wrote:

In D126694#3637690, @ChuanqiXu wrote:

In D126694#3635207, @iains wrote:

@rsmith, @ChuanqiXu apologies for the multiple revisions, this has turned out to be much more involved than I imagined from the standard's text.

In D126694#3629254, @ChuanqiXu wrote:

In D126694#3629251, @iains wrote:

In D126694#3629094, @ChuanqiXu wrote:

BTW, after I applied the patch, the compiler crashes at https://github.com/ChuanqiXu9/stdmodules.

That link points to a project - is there (say) a gist of the crash information?

Here is the crash log:

this code now compiles without error,

Thanks for looking into it!

My personal plan for this revision is to review the details after we add more large tests (at least we have a more complete std modules implementation and I am trying for it. But I find another bug now). So we might need to wait for a while for this patch. How do you think about it?

Well, the difficulty there is that "add more large tests" is not a very specific objective.
I will be first to say that we can tell that the implementation here is necessary, but we cannot tell if it is sufficient - however, IMO we need to find a more definite way to make progress.

Yeah, clearness matters. Let me try to state my idea clearly:
(1) Make std modules sufficient to be able to cover at least <thread>, <vector>, <string>, <algorithm>, <memory> and <atomic>. So that we could run the tests from cppreference at least.
(2) Make https://github.com/alibaba/async_simple/tree/CXX20Modules compilable by clang trunk and std modules

Then we could use (1) or (2) to check some relatively large patches (including this one but not limited). In this way, I feel like we have more confident to say the C++20 Modules in Clang is workable. (Tests is never enough. I know). I would like to see how far I can go in the next month. So I would like to give a more specific answer what test ((1) or (2) or others) we need to use. I think one month might not be super long time in this area. How do you think about it?

---

BTW, I've sent two patches to enhance the usability: D128974 and D129068. And I am fighting with the other problems.

[iains]

rebase, update testcases for upstream changes.

[iains]

rebased and reworked.

The version here has now been tested to consume all of the libc++ headers including
those in experimental and ext.

[ChuanqiXu]

I haven't looked into the details. But I feel the feature worth a sentence in ReleaseNotes. (Maybe in the potential-breaking-change section). Also I think it'll be better to have an option -fenable-discarding-unreachable-decls-in-gmf to control the behavior. It'll be helpful for debugging if we missed some point.

[ChuanqiXu]

Now I think the feature may be important for the performance of modules. And I feel we should work on the ASTWriter side instead of ASTReader side. Since the size of BMIs is a problem now also it shows that it is not free to load the large BMIs. So while it is semantical correct to work on the reader side, it is better for the performance to work on the writer side.

I'd like to finish the idea. And for the current patch, I'd like to refactor it a little bit:

1. Test it by unittest instead of by matching the dump result.
2. Remove the Serialization part. So it will be a NFC patch.
3. Some other trivial polishment.

Of course, I'll still mark you as the author.

How do you feel about this?

[iains]

In D126694#4470139, @ChuanqiXu wrote:

Now I think the feature may be important for the performance of modules. And I feel we should work on the ASTWriter side instead of ASTReader side. Since the size of BMIs is a problem now also it shows that it is not free to load the large BMIs. So while it is semantical correct to work on the reader side, it is better for the performance to work on the writer side.

I'd like to finish the idea. And for the current patch, I'd like to refactor it a little bit:

1. Test it by unittest instead of by matching the dump result.

fine with me

2. Remove the Serialization part. So it will be a NFC patch.

how do you plan to identify "elided" decls (we agreed to leave them in the BMI to keep diagnostics quality up, but we need to be able to ignore them)

3. Some other trivial polishment.

4. I wanted to take another look at using visitors to implement the checks.

Of course, I'll still mark you as the author.

How do you feel about this?

Do you plan to try this before clang-17?
(if so, then go ahead - my next priority for 17 is the lookup bug)

Otherwise, this is on my TODO for clang-18.

BTW, I think that there are other opportunities to reduce the BMI size that we could realistically aim for clang-18
(but let us make that discussion separately from this patch)

[ChuanqiXu]

In D126694#4470235, @iains wrote:

In D126694#4470139, @ChuanqiXu wrote:

Now I think the feature may be important for the performance of modules. And I feel we should work on the ASTWriter side instead of ASTReader side. Since the size of BMIs is a problem now also it shows that it is not free to load the large BMIs. So while it is semantical correct to work on the reader side, it is better for the performance to work on the writer side.

I'd like to finish the idea. And for the current patch, I'd like to refactor it a little bit:

1. Test it by unittest instead of by matching the dump result.

fine with me

2. Remove the Serialization part. So it will be a NFC patch.

how do you plan to identify "elided" decls (we agreed to leave them in the BMI to keep diagnostics quality up, but we need to be able to ignore them)

No. Now I feel it is not good to keep these redundant things in the BMI. They slow down the performance. The enlarge the size of the BMI. They make the model more complex. They've brought a lot of pain points but they bring few benefits. I really don't think we should keep them. And this is the reason why I re-looked at this.

3. Some other trivial polishment.

4. I wanted to take another look at using visitors to implement the checks.

Yeah, this is what I called polishment.

Of course, I'll still mark you as the author.

How do you feel about this?

Do you plan to try this before clang-17?
(if so, then go ahead - my next priority for 17 is the lookup bug)

Otherwise, this is on my TODO for clang-18.

I want to give it a try for clang-17. The direct motivation is that I found the performance of the std module is worse than the direct #include in a small program (~20 lines). After many analysising, I think this one may be the most important technique to improve that. But it is only possible if we elide them in the BMI when writing. It doesn't help if we still keep them in the BMI.

And I feel it pretty bad. This is the reason why I want to make it into clang-17.

BTW, I think that there are other opportunities to reduce the BMI size that we could realistically aim for clang-18
(but let us make that discussion separately from this patch)

Yeah. I tried to look at it a little bit but it looks not so straight forward. A main part I found now is the source managers in serializer. But let's discuss it separately.

[ChuanqiXu]

BTW, in my experience for talking about modules to users, they mainly/mostly care about the compilation performance. And I can't image how many people would like to use modules if they know they won't get a compilation performance win.

[iains]

In D126694#4470250, @ChuanqiXu wrote:

BTW, in my experience for talking about modules to users, they mainly/mostly care about the compilation performance. And I can't image how many people would like to use modules if they know they won't get a compilation performance win.

That is clearly a big motivation - I will ask the folks we were talking to at WG21 if that is their priority - or maybe they care about language isolation etc.

[ChuanqiXu]

That is clearly a big motivation - I will ask the folks we were talking to at WG21 if that is their priority - or maybe they care about language isolation etc.

Yeah, I know the folks in WG21 prefer the language isolation. But you know, there are many folks who are not in WG21...

Oh, the thing I want to say, in this case we have a chance to improve the compilation speed significantly and the so called diagnose quality became a blocker for us. Also it is beneficial to remove them out of the BMI to improve the language isolation feature.

[iains]

In D126694#4470261, @ChuanqiXu wrote:

That is clearly a big motivation - I will ask the folks we were talking to at WG21 if that is their priority - or maybe they care about language isolation etc.

Yeah, I know the folks in WG21 prefer the language isolation. But you know, there are many folks who are not in WG21...

indeed :)

Oh, the thing I want to say, in this case we have a chance to improve the compilation speed significantly and the so called diagnose quality became a blocker for us. Also it is beneficial to remove them out of the BMI to improve the language isolation feature.

Yes, that was the decision at the last time we looked - because removing decls would degrade this - if we have new information that changes our preferred design, then fine.
One solution is to place the elision behind a flag so that the user can choose slower compilation with better diagnostics or faster compilation but maybe harder-to-find errors?

[ChuanqiXu]

Yes, that was the decision at the last time we looked - because removing decls would degrade this - if we have new information that changes our preferred design, then fine.

I remember the major reason for the last time to not remove the decls are that the design of AST doesn't support to remove decls. And my current idea is, we can refuse to write the discardable Decls into the BMIs.

One solution is to place the elision behind a flag so that the user can choose slower compilation with better diagnostics or faster compilation but maybe harder-to-find errors?

I proposed to add a flag. But that was a helper for developers to find if we did anything wrong. I don't want to provide such a flag since on the one hand, there are already many flags and on the other hand, form my user experience, it is not so hard to find the unexported decls. It is really much much more easier than template programming.

[iains]

In D126694#4470297, @ChuanqiXu wrote:

Yes, that was the decision at the last time we looked - because removing decls would degrade this - if we have new information that changes our preferred design, then fine.

I remember the major reason for the last time to not remove the decls are that the design of AST doesn't support to remove decls. And my current idea is, we can refuse to write the discardable Decls into the BMIs.

@rsmith pointed out that the API was not intended for that purpose - so, yes, you are correct that the next place to look was in the serialisation [but ISTR that this also has some challenges because of the way in which the structures are interconnected]. It might be necessary to do a pass before the serialisation and actually prune the AST there. (in a similar manner we'd need to prune it to remove non-inline function bodies in some future version)

One solution is to place the elision behind a flag so that the user can choose slower compilation with better diagnostics or faster compilation but maybe harder-to-find errors?

I proposed to add a flag. But that was a helper for developers to find if we did anything wrong. I don't want to provide such a flag since on the one hand, there are already many flags and on the other hand, form my user experience, it is not so hard to find the unexported decls. It is really much much more easier than template programming.

Yeah, (as you know) I definitely prefer not to add more flags - there are too many - it was only an option in case there were many people against degrading diagnostic output.

[ChuanqiXu]

In D126694#4470358, @iains wrote:

In D126694#4470297, @ChuanqiXu wrote:

Yes, that was the decision at the last time we looked - because removing decls would degrade this - if we have new information that changes our preferred design, then fine.

I remember the major reason for the last time to not remove the decls are that the design of AST doesn't support to remove decls. And my current idea is, we can refuse to write the discardable Decls into the BMIs.

@rsmith pointed out that the API was not intended for that purpose - so, yes, you are correct that the next place to look was in the serialisation [but ISTR that this also has some challenges because of the way in which the structures are interconnected]. It might be necessary to do a pass before the serialisation and actually prune the AST there. (in a similar manner we'd need to prune it to remove non-inline function bodies in some future version)

Yeah, I just feel it is implementable. But I need to give it a try.

One solution is to place the elision behind a flag so that the user can choose slower compilation with better diagnostics or faster compilation but maybe harder-to-find errors?

I proposed to add a flag. But that was a helper for developers to find if we did anything wrong. I don't want to provide such a flag since on the one hand, there are already many flags and on the other hand, form my user experience, it is not so hard to find the unexported decls. It is really much much more easier than template programming.

Yeah, (as you know) I definitely prefer not to add more flags - there are too many - it was only an option in case there were many people against degrading diagnostic output.

Got it.

clang/include/clang/AST/DeclBase.h

enum class ModuleOwnershipKind : unsigned {
 
     /// This declaration has an owning module, and is visible to lookups
     /// that occurs within that module. And it is reachable in other module
     /// when the owning module is transitively imported.
     ReachableWhenImported,
 
     /// This declaration has an owning module, but is only visible to
     /// lookups that occur within that module.
-    /// The discarded declarations in global module fragment belongs
-    /// to this group too.
-    ModulePrivate
+    ModulePrivate,
+
+    /// This declaration is part of a Global Module Fragment, it is permitted
+    /// to discard it and therefore it is not reachable or visible to importers
+    /// of the named module of which the GMF is part.
+    ModuleDiscardable
   };
 
 protected:
   /// The next declaration within the same lexical
   /// DeclContext. These pointers form the linked list that is
   /// traversed via DeclContext's decls_begin()/decls_end().
   ///
   /// The extra three bits are used for the ModuleOwnershipKind.
public:
   bool isReferenced() const;
 
   /// Whether this declaration was referenced. This should not be relied
   /// upon for anything other than debugging.
   bool isThisDeclarationReferenced() const { return Referenced; }
 
   void setReferenced(bool R = true) { Referenced = R; }
 
+  /// Whether this declaration should be retained if it is used.
+  bool isModuleDiscardable() const {
+    return getModuleOwnershipKind() ==
+           Decl::ModuleOwnershipKind::ModuleDiscardable;
+  }
+
   /// Whether this declaration is a top-level declaration (function,
   /// global variable, etc.) that is lexically inside an objc container
   /// definition.
   bool isTopLevelDeclInObjCContainer() const {
     return TopLevelDeclInObjCContainer;
   }
 
   void setTopLevelDeclInObjCContainer(bool V = true) {
     TopLevelDeclInObjCContainer = V;
   }
 
   /// Looks on this and related declarations for an applicable
   /// external source symbol attribute.
   ExternalSourceSymbolAttr *getExternalSourceSymbolAttr() const;
 
   /// Whether this declaration was marked as being private to the
   /// module in which it was defined.
   bool isModulePrivate() const {
     return getModuleOwnershipKind() == ModuleOwnershipKind::ModulePrivate;
   }
 
   /// Whether this declaration was exported in a lexical context.
   /// e.g.:
   ///
   ///   export namespace A {
   ///      void f1();        // isInExportDeclContext() == true
   ///   }
   ///   void A::f1();        // isInExportDeclContext() == false
   ///
   ///   namespace B {
   ///      void f2();        // isInExportDeclContext() == false
   ///   }
   ///   export void B::f2(); // isInExportDeclContext() == true
   bool isInExportDeclContext() const;
 
   bool isInvisibleOutsideTheOwningModule() const {
     return getModuleOwnershipKind() > ModuleOwnershipKind::VisibleWhenImported;
   }
 
-  /// FIXME: Implement discarding declarations actually in global module
-  /// fragment. See [module.global.frag]p3,4 for details.
-  bool isDiscardedInGlobalModuleFragment() const { return false; }
+  /// See [module.global.frag]p3,4 for details.
+  bool isDiscardedInGlobalModuleFragment() const {
+    return getModuleOwnershipKind() == ModuleOwnershipKind::ModuleDiscardable;
+  }
 
   /// Return true if this declaration has an attribute which acts as
   /// definition of the entity, such as 'alias' or 'ifunc'.
   bool hasDefiningAttr() const;
 
   /// Return this declaration's defining attribute if it has one.
   const Attr *getDefiningAttr() const;
 

clang/include/clang/Sema/Sema.h

private:
 
   /// Cache for module units which is usable for current module.
   llvm::DenseSet<const Module *> UsableModuleUnitsCache;
 
   bool isUsableModule(const Module *M);
 
   bool isAcceptableSlow(const NamedDecl *D, AcceptableKind Kind);
 
+  /// When checking for Global Module Fragment decls that need to be made
+  /// reachable we also need to consider the types used.  This set is used to
+  /// ensure that we only visit each type once.
+  llvm::SmallPtrSet<const clang::Type *, 32> GMFTypesVisited;
+
+  /// Examine a type, unwrapping any decls that might be in the GMF.
+  void findGMFReachableDeclsForType(const QualType &Q);
+
+  /// Walk a statement/expression tree looking for cases that require GMF decls
+  /// to be made reachable.
+  void findGMFReachableDeclExprs(clang::Stmt *S);
+
+  /// Starting from a decl D mark any additional decls in the GMF this makes
+  /// reachable.
+  void markGMFDeclsReachableFrom(Decl *D);
+
+  // We have marked some decl as used (or referenced) and therefore (if we are
+  // in a module purview, and there is a GMF) we need to see if that use or
+  // reference requires us to make GMF decl(s) reachable.
+  void handleGMFDeclsReachableFrom(Decl *D) {
+    // Try to use an early exit for the cases we do not need to consider.
+    if (!getLangOpts().CPlusPlusModules || !GlobalModuleFragment ||
+        !isCurrentModulePurview())
+      return;
+
+    // We only need to make the decls reachable, not visible.
+    markGMFDeclsReachableFrom(D);
+  }
+
 public:
+  /// To implement C++20 GMF elision semantics, we initially mark decls in the
+  /// Global Module Fragment as "ModuleDiscardable".  If the decl is then used
+  /// within the module purview, we reset that to visible.
+  void setDeclIsUsed(Decl *D) {
+    D->setIsUsed();
+    handleGMFDeclsReachableFrom(D);
+  }
+
+  /// Likewise when the decl is marked used and notifies mutation listeners.
+  void markDeclUsed(Decl *D, ASTContext &C) {
+    D->markUsed(C);
+    handleGMFDeclsReachableFrom(D);
+  }
+
+  void setDeclReferenced(Decl *D, bool R = true) {
+    D->setReferenced(R);
+    handleGMFDeclsReachableFrom(D);
+  }
+
   /// Get the module unit whose scope we are currently within.
   Module *getCurrentModule() const {
     return ModuleScopes.empty() ? nullptr : ModuleScopes.back().Module;
   }
 
   /// Is the module scope we are an interface?
   bool currentModuleIsInterface() const {
     return ModuleScopes.empty() ? false : ModuleScopes.back().ModuleInterface;

clang/lib/AST/Decl.cpp

 }
 
 static bool isExportedFromModuleInterfaceUnit(const NamedDecl *D) {
   // FIXME: Handle isModulePrivate.
   switch (D->getModuleOwnershipKind()) {
   case Decl::ModuleOwnershipKind::Unowned:
   case Decl::ModuleOwnershipKind::ReachableWhenImported:
   case Decl::ModuleOwnershipKind::ModulePrivate:
+  case Decl::ModuleOwnershipKind::ModuleDiscardable:
     return false;
   case Decl::ModuleOwnershipKind::Visible:
   case Decl::ModuleOwnershipKind::VisibleWhenImported:
     return isInModulePurview(D);
   }
   llvm_unreachable("unexpected module ownership kind");
 }
 

clang/lib/AST/TextNodeDumper.cpp

 //===--- TextNodeDumper.cpp - Printing of AST nodes -----------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements AST dumping of components of individual AST nodes.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/TextNodeDumper.h"
 #include "clang/AST/APValue.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclOpenMP.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/LocInfoType.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
#define DECL(DERIVED, BASE)                                                    \
   case Decl::DERIVED:                                                          \
     return dumpPreviousDeclImpl(OS, cast<DERIVED##Decl>(D));
 #define ABSTRACT_DECL(DECL)
 #include "clang/AST/DeclNodes.inc"
   }
   llvm_unreachable("Decl that isn't part of DeclNodes.inc!");
 }
 
+static void dumpModuleOwnership(raw_ostream &OS, const Decl *D) {
+  switch (D->getModuleOwnershipKind()) {
+  case Decl::ModuleOwnershipKind::Unowned:
+    // OS << " Unowned";
+    break;
+  case Decl::ModuleOwnershipKind::Visible:
+    OS << " Visible";
+    break;
+  case Decl::ModuleOwnershipKind::VisibleWhenImported:
+    OS << " VisibleWhenImported";
+    break;
+  case Decl::ModuleOwnershipKind::ReachableWhenImported:
+    OS << " ReachableWhenImported";
+    break;
+  case Decl::ModuleOwnershipKind::ModulePrivate:
+    OS << " __module_private__";
+    break;
+  case Decl::ModuleOwnershipKind::ModuleDiscardable:
+    OS << " ModuleDiscardable";
+    break;
+  }
+}
+
 TextNodeDumper::TextNodeDumper(raw_ostream &OS, const ASTContext &Context,
                                bool ShowColors)
     : TextTreeStructure(OS, ShowColors), OS(OS), ShowColors(ShowColors),
       Context(&Context), SM(&Context.getSourceManager()),
       PrintPolicy(Context.getPrintingPolicy()),
       Traits(&Context.getCommentCommandTraits()) {}
 
 TextNodeDumper::TextNodeDumper(raw_ostream &OS, bool ShowColors)
if (auto N = T->getNumExpansions())
     OS << " expansions " << *N;
 }
 
 void TextNodeDumper::VisitLabelDecl(const LabelDecl *D) { dumpName(D); }
 
 void TextNodeDumper::VisitTypedefDecl(const TypedefDecl *D) {
   dumpName(D);
   dumpType(D->getUnderlyingType());
-  if (D->isModulePrivate())
-    OS << " __module_private__";
+  dumpModuleOwnership(OS, D);
 }
 
 void TextNodeDumper::VisitEnumDecl(const EnumDecl *D) {
   if (D->isScoped()) {
     if (D->isScopedUsingClassTag())
       OS << " class";
     else
       OS << " struct";
   }
   dumpName(D);
-  if (D->isModulePrivate())
-    OS << " __module_private__";
+  dumpModuleOwnership(OS, D);
   if (D->isFixed())
     dumpType(D->getIntegerType());
 }
 
 void TextNodeDumper::VisitRecordDecl(const RecordDecl *D) {
   OS << ' ' << D->getKindName();
   dumpName(D);
-  if (D->isModulePrivate())
-    OS << " __module_private__";
+  dumpModuleOwnership(OS, D);
   if (D->isCompleteDefinition())
     OS << " definition";
 }
 
 void TextNodeDumper::VisitEnumConstantDecl(const EnumConstantDecl *D) {
   dumpName(D);
   dumpType(D->getType());
 }
 
 void TextNodeDumper::VisitIndirectFieldDecl(const IndirectFieldDecl *D) {
void TextNodeDumper::VisitFunctionDecl(const FunctionDecl *D) {
 
   StorageClass SC = D->getStorageClass();
   if (SC != SC_None)
     OS << ' ' << VarDecl::getStorageClassSpecifierString(SC);
   if (D->isInlineSpecified())
     OS << " inline";
   if (D->isVirtualAsWritten())
     OS << " virtual";
-  if (D->isModulePrivate())
-    OS << " __module_private__";
+  dumpModuleOwnership(OS, D);
 
   if (D->isPure())
     OS << " pure";
   if (D->isDefaulted()) {
     OS << " default";
     if (D->isDeleted())
       OS << "_delete";
   }
   if (D->isDeletedAsWritten())
     OS << " delete";
void TextNodeDumper::VisitLifetimeExtendedTemporaryDecl(
   }
 }
 
 void TextNodeDumper::VisitFieldDecl(const FieldDecl *D) {
   dumpName(D);
   dumpType(D->getType());
   if (D->isMutable())
     OS << " mutable";
-  if (D->isModulePrivate())
-    OS << " __module_private__";
+  dumpModuleOwnership(OS, D);
 }
 
 void TextNodeDumper::VisitVarDecl(const VarDecl *D) {
   dumpName(D);
   dumpType(D->getType());
   StorageClass SC = D->getStorageClass();
   if (SC != SC_None)
     OS << ' ' << VarDecl::getStorageClassSpecifierString(SC);
   switch (D->getTLSKind()) {
   case VarDecl::TLS_None:
     break;
   case VarDecl::TLS_Static:
     OS << " tls";
     break;
   case VarDecl::TLS_Dynamic:
     OS << " tls_dynamic";
     break;
   }
-  if (D->isModulePrivate())
-    OS << " __module_private__";
+  dumpModuleOwnership(OS, D);
   if (D->isNRVOVariable())
     OS << " nrvo";
   if (D->isInline())
     OS << " inline";
   if (D->isConstexpr())
     OS << " constexpr";
   if (D->hasInit()) {
     switch (D->getInitStyle()) {
     case VarDecl::CInit:
       OS << " cinit";
void TextNodeDumper::VisitCXXRecordDecl(const CXXRecordDecl *D) {
   VisitRecordDecl(D);
   if (!D->isCompleteDefinition())
     return;
 
   AddChild([=] {
     {
       ColorScope Color(OS, ShowColors, DeclKindNameColor);
       OS << "DefinitionData";
+      dumpModuleOwnership(OS, D);
     }
 #define FLAG(fn, name)                                                         \
   if (D->fn())                                                                 \
     OS << " " #name;
     FLAG(isParsingBaseSpecifiers, parsing_base_specifiers);
 
     FLAG(isGenericLambda, generic);
     FLAG(isLambda, lambda);
AddChild([=] {
       }
       FLAG(hasDefaultConstructor, exists);
       FLAG(hasTrivialDefaultConstructor, trivial);
       FLAG(hasNonTrivialDefaultConstructor, non_trivial);
       FLAG(hasUserProvidedDefaultConstructor, user_provided);
       FLAG(hasConstexprDefaultConstructor, constexpr);
       FLAG(needsImplicitDefaultConstructor, needs_implicit);
       FLAG(defaultedDefaultConstructorIsConstexpr, defaulted_is_constexpr);
+      dumpModuleOwnership(OS, D);
     });
 
     AddChild([=] {
       {
         ColorScope Color(OS, ShowColors, DeclKindNameColor);
         OS << "CopyConstructor";
       }
       FLAG(hasSimpleCopyConstructor, simple);
       FLAG(hasTrivialCopyConstructor, trivial);
       FLAG(hasNonTrivialCopyConstructor, non_trivial);
       FLAG(hasUserDeclaredCopyConstructor, user_declared);
       FLAG(hasCopyConstructorWithConstParam, has_const_param);
       FLAG(needsImplicitCopyConstructor, needs_implicit);
       FLAG(needsOverloadResolutionForCopyConstructor,
            needs_overload_resolution);
       if (!D->needsOverloadResolutionForCopyConstructor())
         FLAG(defaultedCopyConstructorIsDeleted, defaulted_is_deleted);
       FLAG(implicitCopyConstructorHasConstParam, implicit_has_const_param);
+      dumpModuleOwnership(OS, D);
     });
 
     AddChild([=] {
       {
         ColorScope Color(OS, ShowColors, DeclKindNameColor);
         OS << "MoveConstructor";
       }
       FLAG(hasMoveConstructor, exists);
       FLAG(hasSimpleMoveConstructor, simple);
       FLAG(hasTrivialMoveConstructor, trivial);
       FLAG(hasNonTrivialMoveConstructor, non_trivial);
       FLAG(hasUserDeclaredMoveConstructor, user_declared);
       FLAG(needsImplicitMoveConstructor, needs_implicit);
       FLAG(needsOverloadResolutionForMoveConstructor,
            needs_overload_resolution);
       if (!D->needsOverloadResolutionForMoveConstructor())
         FLAG(defaultedMoveConstructorIsDeleted, defaulted_is_deleted);
+      dumpModuleOwnership(OS, D);
     });
 
     AddChild([=] {
       {
         ColorScope Color(OS, ShowColors, DeclKindNameColor);
         OS << "CopyAssignment";
       }
       FLAG(hasSimpleCopyAssignment, simple);
       FLAG(hasTrivialCopyAssignment, trivial);
       FLAG(hasNonTrivialCopyAssignment, non_trivial);
       FLAG(hasCopyAssignmentWithConstParam, has_const_param);
       FLAG(hasUserDeclaredCopyAssignment, user_declared);
       FLAG(needsImplicitCopyAssignment, needs_implicit);
       FLAG(needsOverloadResolutionForCopyAssignment, needs_overload_resolution);
       FLAG(implicitCopyAssignmentHasConstParam, implicit_has_const_param);
+      dumpModuleOwnership(OS, D);
     });
 
     AddChild([=] {
       {
         ColorScope Color(OS, ShowColors, DeclKindNameColor);
         OS << "MoveAssignment";
       }
       FLAG(hasMoveAssignment, exists);
       FLAG(hasSimpleMoveAssignment, simple);
       FLAG(hasTrivialMoveAssignment, trivial);
       FLAG(hasNonTrivialMoveAssignment, non_trivial);
       FLAG(hasUserDeclaredMoveAssignment, user_declared);
       FLAG(needsImplicitMoveAssignment, needs_implicit);
       FLAG(needsOverloadResolutionForMoveAssignment, needs_overload_resolution);
+      dumpModuleOwnership(OS, D);
     });
 
     AddChild([=] {
       {
         ColorScope Color(OS, ShowColors, DeclKindNameColor);
         OS << "Destructor";
       }
       FLAG(hasSimpleDestructor, simple);
       FLAG(hasIrrelevantDestructor, irrelevant);
       FLAG(hasTrivialDestructor, trivial);
       FLAG(hasNonTrivialDestructor, non_trivial);
       FLAG(hasUserDeclaredDestructor, user_declared);
       FLAG(hasConstexprDestructor, constexpr);
       FLAG(needsImplicitDestructor, needs_implicit);
       FLAG(needsOverloadResolutionForDestructor, needs_overload_resolution);
       if (!D->needsOverloadResolutionForDestructor())
         FLAG(defaultedDestructorIsDeleted, defaulted_is_deleted);
+      dumpModuleOwnership(OS, D);
     });
   });
 
   for (const auto &I : D->bases()) {
     AddChild([=] {
       if (I.isVirtual())
         OS << "virtual ";
       dumpAccessSpecifier(I.getAccessSpecifier());
       dumpType(I.getType());
       if (I.isPackExpansion())
         OS << "...";
+      dumpModuleOwnership(OS, D);
     });
   }
 }
 
 void TextNodeDumper::VisitFunctionTemplateDecl(const FunctionTemplateDecl *D) {
   dumpName(D);
+  dumpModuleOwnership(OS, D);
 }
 
 void TextNodeDumper::VisitClassTemplateDecl(const ClassTemplateDecl *D) {
   dumpName(D);
+  dumpModuleOwnership(OS, D);
 }
 
 void TextNodeDumper::VisitVarTemplateDecl(const VarTemplateDecl *D) {
   dumpName(D);
+  dumpModuleOwnership(OS, D);
 }
 
 void TextNodeDumper::VisitBuiltinTemplateDecl(const BuiltinTemplateDecl *D) {
   dumpName(D);
 }
 
 void TextNodeDumper::VisitTemplateTypeParmDecl(const TemplateTypeParmDecl *D) {
   if (const auto *TC = D->getTypeConstraint()) {
     OS << " ";
     dumpBareDeclRef(TC->getNamedConcept());
     if (TC->getNamedConcept() != TC->getFoundDecl()) {
       OS << " (";
       dumpBareDeclRef(TC->getFoundDecl());
       OS << ")";
     }
   } else if (D->wasDeclaredWithTypename())
     OS << " typename";
   else
     OS << " class";
   OS << " depth " << D->getDepth() << " index " << D->getIndex();
   if (D->isParameterPack())
     OS << " ...";
   dumpName(D);
+  dumpModuleOwnership(OS, D);
 }
 
 void TextNodeDumper::VisitNonTypeTemplateParmDecl(
     const NonTypeTemplateParmDecl *D) {
   dumpType(D->getType());
   OS << " depth " << D->getDepth() << " index " << D->getIndex();
   if (D->isParameterPack())
     OS << " ...";
   dumpName(D);
+  dumpModuleOwnership(OS, D);
 }
 
 void TextNodeDumper::VisitTemplateTemplateParmDecl(
     const TemplateTemplateParmDecl *D) {
   OS << " depth " << D->getDepth() << " index " << D->getIndex();
   if (D->isParameterPack())
     OS << " ...";
   dumpName(D);
+  dumpModuleOwnership(OS, D);
 }
 
 void TextNodeDumper::VisitUsingDecl(const UsingDecl *D) {
   OS << ' ';
   if (D->getQualifier())
     D->getQualifier()->print(OS, D->getASTContext().getPrintingPolicy());
   OS << D->getDeclName();
+  dumpModuleOwnership(OS, D);
 }
 
 void TextNodeDumper::VisitUsingEnumDecl(const UsingEnumDecl *D) {
   OS << ' ';
   dumpBareDeclRef(D->getEnumDecl());
 }
 
 void TextNodeDumper::VisitUnresolvedUsingTypenameDecl(
if (D->getQualifier())
     D->getQualifier()->print(OS, D->getASTContext().getPrintingPolicy());
   OS << D->getDeclName();
   dumpType(D->getType());
 }
 
 void TextNodeDumper::VisitUsingShadowDecl(const UsingShadowDecl *D) {
   OS << ' ';
   dumpBareDeclRef(D->getTargetDecl());
+  dumpModuleOwnership(OS, D);
 }
 
 void TextNodeDumper::VisitConstructorUsingShadowDecl(
     const ConstructorUsingShadowDecl *D) {
   if (D->constructsVirtualBase())
     OS << " virtual";
 
   AddChild([=] {

 void TextNodeDumper::VisitAccessSpecDecl(const AccessSpecDecl *D) {
   OS << ' ';
   dumpAccessSpecifier(D->getAccess());
 }
 
 void TextNodeDumper::VisitFriendDecl(const FriendDecl *D) {
   if (TypeSourceInfo *T = D->getFriendType())
     dumpType(T->getType());
+  dumpModuleOwnership(OS, D);
 }
 
 void TextNodeDumper::VisitObjCIvarDecl(const ObjCIvarDecl *D) {
   dumpName(D);
   dumpType(D->getType());
   if (D->getSynthesize())
     OS << " synthesize";
 

clang/lib/Sema/Sema.cpp

 
 /// ActOnEndOfTranslationUnit - This is called at the very end of the
 /// translation unit when EOF is reached and all but the top-level scope is
 /// popped.
 void Sema::ActOnEndOfTranslationUnit() {
   assert(DelayedDiagnostics.getCurrentPool() == nullptr
          && "reached end of translation unit with a pool attached?");
 
+  //  We have finished checking if decls in a GMF need to be made reachable.
+  GMFTypesVisited.clear();
+
   // If code completion is enabled, don't perform any end-of-translation-unit
   // work.
   if (PP.isCodeCompletionEnabled())
     return;
 
   // Complete translation units and modules define vtables and perform implicit
   // instantiations. PCH files do not.
   if (TUKind != TU_Prefix) {
     DiagnoseUseOfUnimplementedSelectors();
 
     ActOnEndOfTranslationUnitFragment(
         !ModuleScopes.empty() && ModuleScopes.back().Module->Kind ==
                                      Module::PrivateModuleFragment
             ? TUFragmentKind::Private
             : TUFragmentKind::Normal);
 
     if (LateTemplateParserCleanup)
       LateTemplateParserCleanup(OpaqueParser);
 
     CheckDelayedMemberExceptionSpecs();
   } else {
     // If we are building a TU prefix for serialization, it is safe to transfer
     // these over, even though they are not parsed. The end of the TU should be
     // outside of any eager template instantiation scope, so when this AST is
     // deserialized, these templates will not be parsed until the end of the
     // combined TU.
     PendingInstantiations.insert(PendingInstantiations.end(),
                                  LateParsedInstantiations.begin(),
                                  LateParsedInstantiations.end());
     LateParsedInstantiations.clear();
 
     if (LangOpts.PCHInstantiateTemplates) {
       llvm::TimeTraceScope TimeScope("PerformPendingInstantiations");
       PerformPendingInstantiations();
     }
   }
 
   DiagnoseUnterminatedPragmaAlignPack();
   DiagnoseUnterminatedPragmaAttribute();
   DiagnoseUnterminatedOpenMPDeclareTarget();
 
   // All delayed member exception specs should be checked or we end up accepting
   // incompatible declarations.

clang/lib/Sema/SemaDecl.cpp

bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old,
   mergeDeclAttributes(New, Old);
 
   // Merge "pure" flag.
   if (Old->isPure())
     New->setPure();
 
   // Merge "used" flag.
   if (Old->getMostRecentDecl()->isUsed(false))
-    New->setIsUsed();
+    setDeclIsUsed(New);
 
   // Merge attributes from the parameters.  These can mismatch with K&R
   // declarations.
   if (New->getNumParams() == Old->getNumParams())
       for (unsigned i = 0, e = New->getNumParams(); i != e; ++i) {
         ParmVarDecl *NewParam = New->getParamDecl(i);
         ParmVarDecl *OldParam = Old->getParamDecl(i);
         mergeParamDeclAttributes(NewParam, OldParam, *this);
if (haveIncompatibleLanguageLinkages(Old, New)) {
     Diag(New->getLocation(), diag::err_different_language_linkage) << New;
     Diag(OldLocation, PrevDiag);
     New->setInvalidDecl();
     return;
   }
 
   // Merge "used" flag.
   if (Old->getMostRecentDecl()->isUsed(false))
-    New->setIsUsed();
+    setDeclIsUsed(New);
 
   // Keep a chain of previous declarations.
   New->setPreviousDecl(Old);
   if (NewTemplate)
     NewTemplate->setPreviousDecl(OldTemplate);
 
   // Inherit access appropriately.
   New->setAccess(Old->getAccess());

clang/lib/Sema/SemaDeclAttr.cpp

static void handleAliasAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   // Mark target used to prevent unneeded-internal-declaration warnings.
   if (!S.LangOpts.CPlusPlus) {
     // FIXME: demangle Str for C++, as the attribute refers to the mangled
     // linkage name, not the pre-mangled identifier.
     const DeclarationNameInfo target(&S.Context.Idents.get(Str), AL.getLoc());
     LookupResult LR(S, target, Sema::LookupOrdinaryName);
     if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
       for (NamedDecl *ND : LR)
-        ND->markUsed(S.Context);
+        S.markDeclUsed(ND, S.Context);
   }
 
   D->addAttr(::new (S.Context) AliasAttr(S.Context, AL, Str));
 }
 
 static void handleTLSModelAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   StringRef Model;
   SourceLocation LiteralLoc;

clang/lib/Sema/SemaDeclCXX.cpp

SourceLocation BodyLoc =
         FD->getEndLoc().isValid() ? FD->getEndLoc() : FD->getLocation();
     StmtResult Body =
         DefaultedComparisonSynthesizer(*this, RD, FD, DCK, BodyLoc).build();
     if (Body.isInvalid()) {
       FD->setInvalidDecl();
       return;
     }
     FD->setBody(Body.get());
-    FD->markUsed(Context);
+    markDeclUsed(FD, Context);
   }
 
   // The exception specification is needed because we are defining the
   // function. Note that this will reuse the body we just built.
   ResolveExceptionSpec(UseLoc, FD->getType()->castAs<FunctionProtoType>());
 
   if (ASTMutationListener *L = getASTMutationListener())
     L->CompletedImplicitDefinition(FD);
if (SetCtorInitializers(Constructor, /*AnyErrors=*/false)) {
     Constructor->setInvalidDecl();
     return;
   }
 
   SourceLocation Loc = Constructor->getEndLoc().isValid()
                            ? Constructor->getEndLoc()
                            : Constructor->getLocation();
   Constructor->setBody(new (Context) CompoundStmt(Loc));
-  Constructor->markUsed(Context);
+  markDeclUsed(Constructor, Context);
 
   if (ASTMutationListener *L = getASTMutationListener()) {
     L->CompletedImplicitDefinition(Constructor);
   }
 
   DiagnoseUninitializedFields(*this, Constructor);
 }
 
void Sema::DefineInheritingConstructor(SourceLocation CurrentLocation,
   // initializers replaced.
 
   if (SetCtorInitializers(Constructor, /*AnyErrors*/false, Inits)) {
     Constructor->setInvalidDecl();
     return;
   }
 
   Constructor->setBody(new (Context) CompoundStmt(InitLoc));
-  Constructor->markUsed(Context);
+  markDeclUsed(Constructor, Context);
 
   if (ASTMutationListener *L = getASTMutationListener()) {
     L->CompletedImplicitDefinition(Constructor);
   }
 
   DiagnoseUninitializedFields(*this, Constructor);
 }
 
if (CheckDestructor(Destructor)) {
     Destructor->setInvalidDecl();
     return;
   }
 
   SourceLocation Loc = Destructor->getEndLoc().isValid()
                            ? Destructor->getEndLoc()
                            : Destructor->getLocation();
   Destructor->setBody(new (Context) CompoundStmt(Loc));
-  Destructor->markUsed(Context);
+  markDeclUsed(Destructor, Context);
 
   if (ASTMutationListener *L = getASTMutationListener()) {
     L->CompletedImplicitDefinition(Destructor);
   }
 }
 
 void Sema::CheckCompleteDestructorVariant(SourceLocation CurrentLocation,
                                           CXXDestructorDecl *Destructor) {
void Sema::DefineImplicitCopyAssignment(SourceLocation CurrentLocation,
   StmtResult Body;
   {
     CompoundScopeRAII CompoundScope(*this);
     Body = ActOnCompoundStmt(Loc, Loc, Statements,
                              /*isStmtExpr=*/false);
     assert(!Body.isInvalid() && "Compound statement creation cannot fail");
   }
   CopyAssignOperator->setBody(Body.getAs<Stmt>());
-  CopyAssignOperator->markUsed(Context);
+  markDeclUsed(CopyAssignOperator, Context);
 
   if (ASTMutationListener *L = getASTMutationListener()) {
     L->CompletedImplicitDefinition(CopyAssignOperator);
   }
 }
 
 CXXMethodDecl *Sema::DeclareImplicitMoveAssignment(CXXRecordDecl *ClassDecl) {
   assert(ClassDecl->needsImplicitMoveAssignment());
void Sema::DefineImplicitMoveAssignment(SourceLocation CurrentLocation,
   StmtResult Body;
   {
     CompoundScopeRAII CompoundScope(*this);
     Body = ActOnCompoundStmt(Loc, Loc, Statements,
                              /*isStmtExpr=*/false);
     assert(!Body.isInvalid() && "Compound statement creation cannot fail");
   }
   MoveAssignOperator->setBody(Body.getAs<Stmt>());
-  MoveAssignOperator->markUsed(Context);
+  markDeclUsed(MoveAssignOperator, Context);
 
   if (ASTMutationListener *L = getASTMutationListener()) {
     L->CompletedImplicitDefinition(MoveAssignOperator);
   }
 }
 
 CXXConstructorDecl *Sema::DeclareImplicitCopyConstructor(
                                                     CXXRecordDecl *ClassDecl) {
if (SetCtorInitializers(CopyConstructor, /*AnyErrors=*/false)) {
     CopyConstructor->setInvalidDecl();
   }  else {
     SourceLocation Loc = CopyConstructor->getEndLoc().isValid()
                              ? CopyConstructor->getEndLoc()
                              : CopyConstructor->getLocation();
     Sema::CompoundScopeRAII CompoundScope(*this);
     CopyConstructor->setBody(
         ActOnCompoundStmt(Loc, Loc, None, /*isStmtExpr=*/false).getAs<Stmt>());
-    CopyConstructor->markUsed(Context);
+    markDeclUsed(CopyConstructor, Context);
   }
 
   if (ASTMutationListener *L = getASTMutationListener()) {
     L->CompletedImplicitDefinition(CopyConstructor);
   }
 }
 
 CXXConstructorDecl *Sema::DeclareImplicitMoveConstructor(
if (SetCtorInitializers(MoveConstructor, /*AnyErrors=*/false)) {
     MoveConstructor->setInvalidDecl();
   } else {
     SourceLocation Loc = MoveConstructor->getEndLoc().isValid()
                              ? MoveConstructor->getEndLoc()
                              : MoveConstructor->getLocation();
     Sema::CompoundScopeRAII CompoundScope(*this);
     MoveConstructor->setBody(ActOnCompoundStmt(
         Loc, Loc, None, /*isStmtExpr=*/ false).getAs<Stmt>());
-    MoveConstructor->markUsed(Context);
+    markDeclUsed(MoveConstructor, Context);
   }
 
   if (ASTMutationListener *L = getASTMutationListener()) {
     L->CompletedImplicitDefinition(MoveConstructor);
   }
 }
 
 bool Sema::isImplicitlyDeleted(FunctionDecl *FD) {
void Sema::DefineImplicitLambdaToFunctionPointerConversion(
   // For both the conversion and static-invoker template specializations
   // we construct their body's in this function, so no need to add them
   // to the PendingInstantiations.
   MarkFunctionReferenced(CurrentLocation, CallOp);
 
   // Fill in the __invoke function with a dummy implementation. IR generation
   // will fill in the actual details. Update its type in case it contained
   // an 'auto'.
-  Invoker->markUsed(Context);
+  markDeclUsed(Invoker, Context);
   Invoker->setReferenced();
   Invoker->setType(Conv->getReturnType()->getPointeeType());
   Invoker->setBody(new (Context) CompoundStmt(Conv->getLocation()));
 
   // Construct the body of the conversion function { return __invoke; }.
   Expr *FunctionRef = BuildDeclRefExpr(Invoker, Invoker->getType(),
                                        VK_LValue, Conv->getLocation());
   assert(FunctionRef && "Can't refer to __invoke function?");
   Stmt *Return = BuildReturnStmt(Conv->getLocation(), FunctionRef).get();
   Conv->setBody(CompoundStmt::Create(Context, Return, FPOptionsOverride(),
                                      Conv->getLocation(), Conv->getLocation()));
-  Conv->markUsed(Context);
+  markDeclUsed(Conv, Context);
   Conv->setReferenced();
 
   if (ASTMutationListener *L = getASTMutationListener()) {
     L->CompletedImplicitDefinition(Conv);
     L->CompletedImplicitDefinition(Invoker);
   }
 }
 
if (Return.isInvalid()) {
     Conv->setInvalidDecl();
     return;
   }
 
   // Set the body of the conversion function.
   Stmt *ReturnS = Return.get();
   Conv->setBody(CompoundStmt::Create(Context, ReturnS, FPOptionsOverride(),
                                      Conv->getLocation(), Conv->getLocation()));
-  Conv->markUsed(Context);
+  markDeclUsed(Conv, Context);
 
   // We're done; notify the mutation listener, if any.
   if (ASTMutationListener *L = getASTMutationListener()) {
     L->CompletedImplicitDefinition(Conv);
   }
 }
 
 /// Determine whether the given list arguments contains exactly one

clang/lib/Sema/SemaExpr.cpp

 #include "clang/Sema/DelayedDiagnostic.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Overload.h"
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
+#include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaFixItUtils.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/SaveAndRestore.h"

 ExprResult Sema::ActOnUnaryOp(Scope *S, SourceLocation OpLoc,
                               tok::TokenKind Op, Expr *Input) {
   return BuildUnaryOp(S, OpLoc, ConvertTokenKindToUnaryOpcode(Op), Input);
 }
 
 /// ActOnAddrLabel - Parse the GNU address of label extension: "&&foo".
 ExprResult Sema::ActOnAddrLabel(SourceLocation OpLoc, SourceLocation LabLoc,
                                 LabelDecl *TheDecl) {
-  TheDecl->markUsed(Context);
+  markDeclUsed(TheDecl, Context);
   // Create the AST node.  The address of a label always has type 'void*'.
   return new (Context) AddrLabelExpr(OpLoc, LabLoc, TheDecl,
                                      Context.getPointerType(Context.VoidTy));
 }
 
 void Sema::ActOnStartStmtExpr() {
   PushExpressionEvaluationContext(ExprEvalContexts.back().Context);
 }

 }
 
 /// Mark a function referenced, and check whether it is odr-used
 /// (C++ [basic.def.odr]p2, C99 6.9p3)
 void Sema::MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func,
                                   bool MightBeOdrUse) {
   assert(Func && "No function?");
 
-  Func->setReferenced();
+  setDeclReferenced(Func);
 
   // Recursive functions aren't really used until they're used from some other
   // context.
   bool IsRecursiveCall = CurContext == Func;
 
   // C++11 [basic.def.odr]p3:
   //   A function whose name appears as a potentially-evaluated expression is
   //   odr-used if it is the unique lookup result or the selected member of a
if (OdrUse == OdrUseContext::Used && !Func->isUsed(/*CheckUsedAttr=*/false)) {
     if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
       if (auto *Dtor = dyn_cast<CXXDestructorDecl>(Func)) {
         CXXRecordDecl *Parent = Dtor->getParent();
         if (Parent->getNumVBases() > 0 && !Dtor->getBody())
           CheckCompleteDestructorVariant(Loc, Dtor);
       }
     }
 
-    Func->markUsed(Context);
+    markDeclUsed(Func, Context);
   }
 }
 
 /// Directly mark a variable odr-used. Given a choice, prefer to use
 /// MarkVariableReferenced since it does additional checks and then
 /// calls MarkVarDeclODRUsed.
 /// If the variable must be captured:
 ///  - if FunctionScopeIndexToStopAt is null, capture it in the CurContext
if (VarTarget == Sema::CVT_Host &&
       // externalize the static device side variable ODR-used by host code.
       if (!Var->hasExternalStorage())
         SemaRef.getASTContext().CUDADeviceVarODRUsedByHost.insert(Var);
       else if (SemaRef.LangOpts.GPURelocatableDeviceCode)
         SemaRef.getASTContext().CUDAExternalDeviceDeclODRUsedByHost.insert(Var);
     }
   }
 
-  Var->markUsed(SemaRef.Context);
+  SemaRef.markDeclUsed(Var, SemaRef.Context);
 }
 
 void Sema::MarkCaptureUsedInEnclosingContext(VarDecl *Capture,
                                              SourceLocation Loc,
                                              unsigned CapturingScopeIndex) {
   MarkVarDeclODRUsed(Capture, Loc, *this, &CapturingScopeIndex);
 }
 

 }
 
 static void DoMarkVarDeclReferenced(
     Sema &SemaRef, SourceLocation Loc, VarDecl *Var, Expr *E,
     llvm::DenseMap<const VarDecl *, int> &RefsMinusAssignments) {
   assert((!E || isa<DeclRefExpr>(E) || isa<MemberExpr>(E) ||
           isa<FunctionParmPackExpr>(E)) &&
          "Invalid Expr argument to DoMarkVarDeclReferenced");
-  Var->setReferenced();
+  SemaRef.setDeclReferenced(Var);
 
   if (Var->isInvalidDecl())
     return;
 
   auto *MSI = Var->getMemberSpecializationInfo();
   TemplateSpecializationKind TSK = MSI ? MSI->getTemplateSpecializationKind()
                                        : Var->getTemplateSpecializationKind();
 
if (auto *VD = dyn_cast<VarDecl>(D)) {
       MarkVariableReferenced(Loc, VD);
       return;
     }
   }
   if (auto *FD = dyn_cast<FunctionDecl>(D)) {
     MarkFunctionReferenced(Loc, FD, MightBeOdrUse);
     return;
   }
-  D->setReferenced();
+  setDeclReferenced(D);
 }
 
 namespace {
   // Mark all of the declarations used by a type as referenced.
   // FIXME: Not fully implemented yet! We need to have a better understanding
   // of when we're entering a context we should not recurse into.
   // FIXME: This is and EvaluatedExprMarker are more-or-less equivalent to
   // TreeTransforms rebuilding the type in a new context. Rather than

clang/lib/Sema/SemaInit.cpp

static OverloadingResult TryRefInitWithConversionFunction(
   OverloadCandidateSet::iterator Best;
   if (OverloadingResult Result
         = CandidateSet.BestViableFunction(S, DeclLoc, Best))
     return Result;
 
   FunctionDecl *Function = Best->Function;
   // This is the overload that will be used for this initialization step if we
   // use this initialization. Mark it as referenced.
-  Function->setReferenced();
+  S.setDeclReferenced(Function);
 
   // Compute the returned type and value kind of the conversion.
   QualType cv3T3;
   if (isa<CXXConversionDecl>(Function))
     cv3T3 = Function->getReturnType();
   else
     cv3T3 = T1;
 
if (OverloadingResult Result
     // need the initialization sequence to decide whether to perform the second
     // overload resolution.
     if (!(Result == OR_Deleted &&
           Kind.getKind() == InitializationKind::IK_Copy))
       return;
   }
 
   FunctionDecl *Function = Best->Function;
-  Function->setReferenced();
+  S.setDeclReferenced(Function);
   bool HadMultipleCandidates = (CandidateSet.size() > 1);
 
   if (isa<CXXConstructorDecl>(Function)) {
     // Add the user-defined conversion step. Any cv-qualification conversion is
     // subsumed by the initialization. Per DR5, the created temporary is of the
     // cv-unqualified type of the destination.
     Sequence.AddUserConversionStep(Function, Best->FoundDecl,
                                    DestType.getUnqualifiedType(),

clang/lib/Sema/SemaLambda.cpp

VarDecl *Sema::createLambdaInitCaptureVarDecl(SourceLocation Loc,
   // init-capture.
   // FIXME: Pass in separate source locations for '&' and identifier.
   VarDecl *NewVD = VarDecl::Create(Context, CurContext, Loc,
                                    Loc, Id, InitCaptureType, TSI, SC_Auto);
   NewVD->setInitCapture(true);
   NewVD->setReferenced(true);
   // FIXME: Pass in a VarDecl::InitializationStyle.
   NewVD->setInitStyle(static_cast<VarDecl::InitializationStyle>(InitStyle));
-  NewVD->markUsed(Context);
+  markDeclUsed(static_cast<Decl *>(NewVD), Context);
   NewVD->setInit(Init);
   if (NewVD->isParameterPack())
     getCurLambda()->LocalPacks.push_back(NewVD);
   return NewVD;
 }
 
 void Sema::addInitCapture(LambdaScopeInfo *LSI, VarDecl *Var) {
   assert(Var->isInitCapture() && "init capture flag should be set");
ExprResult Sema::BuildBlockForLambdaConversion(SourceLocation CurrentLocation,
                                                Expr *Src) {
   // Make sure that the lambda call operator is marked used.
   CXXRecordDecl *Lambda = Conv->getParent();
   CXXMethodDecl *CallOperator
     = cast<CXXMethodDecl>(
         Lambda->lookup(
           Context.DeclarationNames.getCXXOperatorName(OO_Call)).front());
   CallOperator->setReferenced();
-  CallOperator->markUsed(Context);
+  markDeclUsed(CallOperator, Context);
 
   ExprResult Init = PerformCopyInitialization(
       InitializedEntity::InitializeLambdaToBlock(ConvLocation, Src->getType()),
       CurrentLocation, Src);
   if (!Init.isInvalid())
     Init = ActOnFinishFullExpr(Init.get(), /*DiscardedValue*/ false);
 
   if (Init.isInvalid())

clang/lib/Sema/SemaModule.cpp

 //===--- SemaModule.cpp - Semantic Analysis for Modules -------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file implements semantic analysis for modules (C++ modules syntax,
 //  Objective-C modules syntax, and Clang header modules).
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/ASTConsumer.h"
+#include "clang/AST/DeclFriend.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/SemaInternal.h"
 
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/Format.h"
+
 using namespace clang;
 using namespace sema;
 
 static void checkModuleImportContext(Sema &S, Module *M,
                                      SourceLocation ImportLoc, DeclContext *DC,
                                      bool FromInclude = false) {
   SourceLocation ExternCLoc;
 
if (!ModuleScopes.empty() &&
     // Under -std=c++2a -fmodules-ts, we can find an explicit 'module;' after
     // already implicitly entering the global module fragment. That's OK.
     assert(getLangOpts().CPlusPlusModules && getLangOpts().ModulesTS &&
            "unexpectedly encountered multiple global module fragment decls");
     ModuleScopes.back().BeginLoc = ModuleLoc;
     return nullptr;
   }
 
-  // We start in the global module; all those declarations are implicitly
-  // module-private (though they do not have module linkage).
   Module *GlobalModule =
       PushGlobalModuleFragment(ModuleLoc, /*IsImplicit=*/false);
 
   // All declarations created from now on are owned by the global module.
   auto *TU = Context.getTranslationUnitDecl();
   // [module.global.frag]p2
   // A global-module-fragment specifies the contents of the global module
   // fragment for a module unit. The global module fragment can be used to
   // provide declarations that are attached to the global module and usable
   // within the module unit.
   //
-  // So the declations in the global module shouldn't be visible by default.
-  TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ReachableWhenImported);
+  // We are expected to elide GMF decls that are not used in the purview of the
+  // named module.  In order to do that, create GMF decls with a marker that is
+  // updated when a decl is used.
+  TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModuleDiscardable);
   TU->setLocalOwningModule(GlobalModule);
 
   // FIXME: Consider creating an explicit representation of this declaration.
   return nullptr;
 }
 
 void Sema::HandleStartOfHeaderUnit() {
   assert(getLangOpts().CPlusPlusModules &&
Diag(Path.front().second, diag::err_current_module_name_mismatch)
         << getLangOpts().CurrentModule;
     return nullptr;
   }
   const_cast<LangOptions&>(getLangOpts()).CurrentModule = ModuleName;
 
   auto &Map = PP.getHeaderSearchInfo().getModuleMap();
   Module *Mod;
 
   switch (MDK) {
   case ModuleDeclKind::Interface:
   case ModuleDeclKind::PartitionInterface: {
     // We can't have parsed or imported a definition of this module or parsed a
     // module map defining it already.
     if (auto *M = Map.findModule(ModuleName)) {
       Diag(Path[0].second, diag::err_module_redefinition) << ModuleName;
       if (M->DefinitionLoc.isValid())
         Diag(M->DefinitionLoc, diag::note_prev_module_definition);
Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
   // Make and return the import decl to be added to the current TU.
   if (MDK == ModuleDeclKind::Implementation) {
     // Make the import decl for the interface.
     ImportDecl *Import =
         ImportDecl::Create(Context, CurContext, ModuleLoc, Mod, Path[0].second);
     // and return it to be added.
     return ConvertDeclToDeclGroup(Import);
   }
 
   // FIXME: Create a ModuleDecl.
   return nullptr;
 }
 
 Sema::DeclGroupPtrTy
 Sema::ActOnPrivateModuleFragmentDecl(SourceLocation ModuleLoc,
                                      SourceLocation PrivateLoc) {
   // C++20 [basic.link]/2:
   //   A private-module-fragment shall appear only in a primary module
   //   interface unit.
if (!D->isInvalidDecl()) {
     SourceLocation BlockStart =
         ED->hasBraces() ? ED->getBeginLoc() : SourceLocation();
     for (auto *Child : ED->decls()) {
       if (checkExportedDecl(*this, Child, BlockStart)) {
         // If a top-level child is a linkage-spec declaration, it might contain
         // no declarations (transitively), in which case it's ill-formed.
         diagExportedUnnamedDecl(*this, UnnamedDeclKind::Context, Child,
                                 BlockStart);
+      } else if (getLangOpts().CPlusPlusModules && GlobalModuleFragment) {
+        // An export must be in module purview, so mark any used GMF content.
+        if (auto *USD = dyn_cast<UsingShadowDecl>(Child)) {
+          NamedDecl *Target = USD->getUnderlyingDecl();
+          if (Target->hasOwningModule() &&
+              Target->getOwningModule()->isGlobalModule()) {
+            // Mark the target of the using declaration as reachable.
+            markGMFDeclsReachableFrom(Target);
+          }
+        }
       }
       if (auto *FD = dyn_cast<FunctionDecl>(Child)) {
         // [dcl.inline]/7
         // If an inline function or variable that is attached to a named module
         // is declared in a definition domain, it shall be defined in that
         // domain.
         // So, if the current declaration does not have a definition, we must
         // check at the end of the TU (or when the PMF starts) to see that we
         // have a definition at that point.
         if (FD->isInlineSpecified() && !FD->isDefined())
           PendingInlineFuncDecls.insert(FD);
       }
     }
   }
 
   return D;
 }
 
 Module *Sema::PushGlobalModuleFragment(SourceLocation BeginLoc,
                                        bool IsImplicit) {
   // We shouldn't create new global module fragment if there is already
   // one.
assert(!ModuleScopes.empty() && getCurrentModule()->isGlobalModule() &&
          "left the wrong module scope, which is not global module fragment");
   ModuleScopes.pop_back();
 }
 
 bool Sema::isModuleUnitOfCurrentTU(const Module *M) const {
   assert(M);
 
   Module *CurrentModuleUnit = getCurrentModule();
 
   // If we are not in a module currently, M must not be the module unit of
   // current TU.
   if (!CurrentModuleUnit)
     return false;
 
   return M->isSubModuleOf(CurrentModuleUnit->getTopLevelModule());
 }
+
+// A helper to process a used type to find GMF reachable decls which it uses.
+
+void Sema::findGMFReachableDeclsForType(const QualType &Q) {
+  // If we have a reference, mark the underlying type.
+  auto QR = Q.getNonReferenceType();
+  auto UT = QR.getDesugaredType(Context);
+  const Type *T = UT.getTypePtr();
+  assert(T && "missing type pointer?");
+  // Do not try to process it if incomplete (allow it to be visited again).
+  if (T->isIncompleteType())
+    return;
+  if (auto *AT = dyn_cast<AutoType>(T)) {
+    if (AT->isDeduced() && !AT->getDeducedType().isNull())
+      findGMFReachableDeclsForType(AT->getDeducedType());
+    // Allow a revisit to this, in the event that the deduced type is unknown
+    return;
+  }
+
+  // Only visit each Type once.
+  if (!GMFTypesVisited.insert(T).second)
+    return;
+  // Return as early as possible if this is not an entity we would need to mark
+  // but typedefs of built-in types need to be caught first (since the typedef
+  // could be in the GMF).
+  if (auto *TD = dyn_cast<TypedefType>(T)) {
+    markGMFDeclsReachableFrom(TD->getDecl());
+  } else if (T->isBuiltinType()) {
+    return;
+  } else if (auto *PT = dyn_cast<PointerType>(T)) {
+    return;
+  } else if (auto *ET = dyn_cast<EnumType>(T)) {
+    markGMFDeclsReachableFrom(ET->getDecl());
+  } else if (auto *TST = dyn_cast<TemplateSpecializationType>(T)) {
+    if (auto *RT = TST->getAsCXXRecordDecl())
+      markGMFDeclsReachableFrom(RT);
+    TemplateDecl *TD = TST->getTemplateName().getAsTemplateDecl();
+    markGMFDeclsReachableFrom(TD);
+  } else if (auto *RD = T->getAsCXXRecordDecl()) {
+    markGMFDeclsReachableFrom(RD);
+  } else if (auto *TP = dyn_cast<TemplateTypeParmType>(T)) {
+    markGMFDeclsReachableFrom(TP->getDecl());
+  } else if (auto *DN = dyn_cast<DependentNameType>(T)) {
+    return;
+  } else if (auto *ST = dyn_cast<SubstTemplateTypeParmType>(T)) {
+    findGMFReachableDeclsForType(ST->getReplacementType());
+  } else if (auto *ET = dyn_cast<ElaboratedType>(T)) {
+    findGMFReachableDeclsForType(ET->getNamedType());
+  } else if (auto *PE = dyn_cast<PackExpansionType>(T)) {
+    findGMFReachableDeclsForType(PE->getPattern());
+  } else if (auto *AT = dyn_cast<ArrayType>(T)) {
+    findGMFReachableDeclsForType(AT->getElementType());
+  } else if (auto *DT = dyn_cast<DecltypeType>(T)) {
+    if (!DT->getUnderlyingType().isNull())
+      findGMFReachableDeclsForType(DT->getUnderlyingType());
+  } else if (auto *UT = dyn_cast<UnaryTransformType>(T)) {
+    if (!UT->getUnderlyingType().isNull())
+      findGMFReachableDeclsForType(UT->getUnderlyingType());
+  } else if (auto *MT = dyn_cast<MemberPointerType>(T)) {
+    findGMFReachableDeclsForType(QualType(MT->getClass(), 0));
+  } else if (auto *DT = dyn_cast<DependentTemplateSpecializationType>(T)) {
+    auto *NNS = DT->getQualifier();
+    if (NNS && NNS->getKind() == NestedNameSpecifier::TypeSpec)
+      findGMFReachableDeclsForType(QualType(NNS->getAsType(), 0));
+  } else {
+    llvm::dbgs() << llvm::format("unhandled type: %p ", T);
+    T->dump();
+  }
+}
+
+// A helper to recurse through a stmt/expr tree finding expressions with
+// content that makes GMF decls reachable.
+
+void Sema::findGMFReachableDeclExprs(clang::Stmt *S) {
+
+  if (DeclRefExpr *DR = dyn_cast<DeclRefExpr>(S)) {
+    markGMFDeclsReachableFrom(DR->getFoundDecl());
+  } else if (DependentScopeDeclRefExpr *DS =
+                 dyn_cast<DependentScopeDeclRefExpr>(S)) {
+    auto *NNS = DS->getQualifier();
+    if (NNS && NNS->getKind() == NestedNameSpecifier::TypeSpec)
+      findGMFReachableDeclsForType(QualType(NNS->getAsType(), 0));
+  } else if (UnresolvedLookupExpr *ULE = dyn_cast<UnresolvedLookupExpr>(S)) {
+    for (auto *D : ULE->decls())
+      markGMFDeclsReachableFrom(D);
+  } else if (DeclStmt *DS = dyn_cast<DeclStmt>(S)) {
+    if (DS->isSingleDecl())
+      markGMFDeclsReachableFrom(DS->getSingleDecl());
+    else {
+      for (auto *D : DS->decls())
+        markGMFDeclsReachableFrom(D);
+    }
+  }
+
+  // Recurse into sub-expressions.
+  for (auto *CH : S->children()) {
+    if (CH)
+      findGMFReachableDeclExprs(CH);
+  }
+}
+
+// Starting from decl Orig (which is marked as reachable/visible if it is in
+// the GMF and currently marked as ModuleDiscardable), process other decls
+// reachable from this one and mark them too.
+
+void Sema::markGMFDeclsReachableFrom(Decl *Orig) {
+
+  // Do not alter the ownership kind unless it is ModuleDiscardable.
+  if (!Orig->isModuleDiscardable())
+    return;
+
+  // Only visit each Decl once, when the state is changed.
+  assert(Orig->getOwningModule() && Orig->getOwningModule()->isGlobalModule() &&
+         "should not have a discardable decl outside the GMF");
+  Orig->setModuleOwnershipKind(
+      Decl::ModuleOwnershipKind::ReachableWhenImported);
+
+  // Now process the decl to determine if any dependent decls should also be
+  // updated.
+
+  if (auto FT = dyn_cast<FunctionTemplateDecl>(Orig)) {
+    markGMFDeclsReachableFrom(FT->getTemplatedDecl());
+    for (auto *SP : FT->specializations())
+      markGMFDeclsReachableFrom(SP);
+    for (auto *TA : *FT->getTemplateParameters())
+      markGMFDeclsReachableFrom(TA);
+    return;
+  }
+
+  if (auto *FD = dyn_cast<FunctionDecl>(Orig)) {
+    findGMFReachableDeclsForType(FD->getReturnType());
+    for (ParmVarDecl *P : FD->parameters())
+      markGMFDeclsReachableFrom(P);
+    const FunctionDecl *BodyDecl;
+    if (auto *S = FD->getBody(BodyDecl))
+      findGMFReachableDeclExprs(S);
+    return;
+  }
+
+  if (auto TS = dyn_cast<ClassTemplateSpecializationDecl>(Orig)) {
+    markGMFDeclsReachableFrom(TS->getSpecializedTemplate());
+    if (!TS->isCompleteDefinition())
+      return;
+    for (auto B : TS->bases())
+      findGMFReachableDeclsForType(B.getType());
+    return;
+  }
+
+  if (auto CD = dyn_cast<ClassTemplateDecl>(Orig)) {
+    markGMFDeclsReachableFrom(CD->getTemplatedDecl());
+    for (auto *SP : CD->specializations())
+      markGMFDeclsReachableFrom(SP);
+    for (auto *TA : *CD->getTemplateParameters())
+      markGMFDeclsReachableFrom(TA);
+    return;
+  }
+
+  if (auto *RD = dyn_cast<CXXRecordDecl>(Orig)) {
+    if (!RD->isCompleteDefinition())
+      return;
+    DeclContext *DC = Decl::castToDeclContext(RD);
+    for (auto *D : DC->decls())
+      markGMFDeclsReachableFrom(D);
+    for (auto B : RD->bases())
+      findGMFReachableDeclsForType(B.getType());
+    return;
+  }
+
+  if (auto *TTA = dyn_cast<TemplateTypeParmDecl>(Orig)) {
+    if (TTA->hasDefaultArgument())
+      findGMFReachableDeclsForType(TTA->getDefaultArgument());
+    return;
+  }
+
+  if (auto *PD = dyn_cast<ParmVarDecl>(Orig)) {
+    findGMFReachableDeclsForType(PD->getOriginalType());
+    if (PD->hasUninstantiatedDefaultArg())
+      // FIXME: maybe we should walk the expression here.
+      return;
+    if (auto *E = PD->getDefaultArg())
+      findGMFReachableDeclExprs(E);
+    return;
+  }
+
+  if (auto *VD = dyn_cast<VarDecl>(Orig)) {
+    findGMFReachableDeclsForType(VD->getType());
+    if (VD->hasInit())
+      findGMFReachableDeclExprs(VD->getInit());
+    return;
+  }
+
+  if (auto *FD = dyn_cast<FieldDecl>(Orig)) {
+    findGMFReachableDeclsForType(FD->getType());
+    if (auto *Init = FD->getInClassInitializer())
+      findGMFReachableDeclExprs(Init);
+    return;
+  }
+
+  if (auto *FD = dyn_cast<FriendDecl>(Orig)) {
+    if (auto *DD = FD->getFriendDecl())
+      markGMFDeclsReachableFrom(DD);
+    return;
+  }
+
+  if (auto *ED = dyn_cast<EnumConstantDecl>(Orig)) {
+    if (auto *E = dyn_cast<EnumDecl>(ED->getDeclContext()))
+      markGMFDeclsReachableFrom(E);
+    return;
+  }
+}

clang/lib/Sema/SemaOpenMP.cpp

static VarDecl *buildVarDecl(Sema &SemaRef, SourceLocation Loc, QualType Type,
   }
   return Decl;
 }
 
 static DeclRefExpr *buildDeclRefExpr(Sema &S, VarDecl *D, QualType Ty,
                                      SourceLocation Loc,
                                      bool RefersToCapture = false) {
   D->setReferenced();
-  D->markUsed(S.Context);
+  S.markDeclUsed(D, S.Context);
   return DeclRefExpr::Create(S.getASTContext(), NestedNameSpecifierLoc(),
                              SourceLocation(), D, RefersToCapture, Loc, Ty,
                              VK_LValue);
 }
 
 void DSAStackTy::addTaskgroupReductionData(const ValueDecl *D, SourceRange SR,
                                            BinaryOperatorKind BOK) {
   D = getCanonicalDecl(D);
Sema::CheckOMPThreadPrivateDecl(SourceLocation Loc, ArrayRef<Expr *> VarList) {
   SmallVector<Expr *, 8> Vars;
   for (Expr *RefExpr : VarList) {
     auto *DE = cast<DeclRefExpr>(RefExpr);
     auto *VD = cast<VarDecl>(DE->getDecl());
     SourceLocation ILoc = DE->getExprLoc();
 
     // Mark variable as used.
     VD->setReferenced();
-    VD->markUsed(Context);
+    markDeclUsed(static_cast<Decl *>(VD), Context);
 
     QualType QType = VD->getType();
     if (QType->isDependentType() || QType->isInstantiationDependentType()) {
       // It will be analyzed later.
       Vars.push_back(DE);
       continue;
     }
 

clang/lib/Sema/SemaStmt.cpp

StmtResult Sema::BuildCXXForRangeStmt(SourceLocation ForLoc,
   VarDecl *LoopVar = cast<VarDecl>(LoopVarDS->getSingleDecl());
 
   StmtResult BeginDeclStmt = Begin;
   StmtResult EndDeclStmt = End;
   ExprResult NotEqExpr = Cond, IncrExpr = Inc;
 
   if (RangeVarType->isDependentType()) {
     // The range is implicitly used as a placeholder when it is dependent.
-    RangeVar->markUsed(Context);
+    markDeclUsed(static_cast<Decl *>(RangeVar), Context);
 
     // Deduce any 'auto's in the loop variable as 'DependentTy'. We'll fill
     // them in properly when we instantiate the loop.
     if (!LoopVar->isInvalidDecl() && Kind != BFRK_Check) {
       if (auto *DD = dyn_cast<DecompositionDecl>(LoopVar))
         for (auto *Binding : DD->bindings())
           Binding->setType(Context.DependentTy);
       LoopVar->setType(SubstAutoTypeDependent(LoopVar->getType()));
StmtResult Sema::FinishCXXForRangeStmt(Stmt *S, Stmt *B) {
 
   return S;
 }
 
 StmtResult Sema::ActOnGotoStmt(SourceLocation GotoLoc,
                                SourceLocation LabelLoc,
                                LabelDecl *TheDecl) {
   setFunctionHasBranchIntoScope();
-  TheDecl->markUsed(Context);
+  markDeclUsed(TheDecl, Context);
   return new (Context) GotoStmt(TheDecl, GotoLoc, LabelLoc);
 }
 
 StmtResult
 Sema::ActOnIndirectGotoStmt(SourceLocation GotoLoc, SourceLocation StarLoc,
                             Expr *E) {
   // Convert operand to void*
   if (!E->isTypeDependent()) {

clang/lib/Sema/SemaStmtAsm.cpp

 LabelDecl *Sema::GetOrCreateMSAsmLabel(StringRef ExternalLabelName,
                                        SourceLocation Location,
                                        bool AlwaysCreate) {
   LabelDecl* Label = LookupOrCreateLabel(PP.getIdentifierInfo(ExternalLabelName),
                                          Location);
 
   if (Label->isMSAsmLabel()) {
     // If we have previously created this label implicitly, mark it as used.
-    Label->markUsed(Context);
+    markDeclUsed(Label, Context);
   } else {
     // Otherwise, insert it, but only resolve it if we have seen the label itself.
     std::string InternalName;
     llvm::raw_string_ostream OS(InternalName);
     // Create an internal name for the label.  The name should not be a valid
     // mangled name, and should be unique.  We use a dot to make the name an
     // invalid mangled name. We use LLVM's inline asm ${:uid} escape so that a
     // unique label is generated each time this blob is emitted, even after

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

Decl *TemplateDeclInstantiator::InstantiateTypedefNameDecl(TypedefNameDecl *D,
   }
 
   SemaRef.InstantiateAttrs(TemplateArgs, D, Typedef);
 
   if (D->getUnderlyingType()->getAs<DependentNameType>())
     SemaRef.inferGslPointerAttribute(Typedef);
 
   Typedef->setAccess(D->getAccess());
-  Typedef->setReferenced(D->isReferenced());
+  SemaRef.setDeclReferenced(Typedef, D->isReferenced());
 
   return Typedef;
 }
 
 Decl *TemplateDeclInstantiator::VisitTypedefDecl(TypedefDecl *D) {
   Decl *Typedef = InstantiateTypedefNameDecl(D, /*IsTypeAlias=*/false);
   if (Typedef)
     Owner->addDecl(Typedef);
TemplateDeclInstantiator::VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) {
   Owner->addDecl(Inst);
 
   return Inst;
 }
 
 Decl *TemplateDeclInstantiator::VisitBindingDecl(BindingDecl *D) {
   auto *NewBD = BindingDecl::Create(SemaRef.Context, Owner, D->getLocation(),
                                     D->getIdentifier());
-  NewBD->setReferenced(D->isReferenced());
+  SemaRef.setDeclReferenced(NewBD, D->isReferenced());
   SemaRef.CurrentInstantiationScope->InstantiatedLocal(D, NewBD);
   return NewBD;
 }
 
 Decl *TemplateDeclInstantiator::VisitDecompositionDecl(DecompositionDecl *D) {
   // Transform the bindings first.
   SmallVector<BindingDecl*, 16> NewBindings;
   for (auto *OldBD : D->bindings())
void Sema::BuildVariableInstantiation(
   NewVar->setConstexpr(OldVar->isConstexpr());
   NewVar->setInitCapture(OldVar->isInitCapture());
   NewVar->setPreviousDeclInSameBlockScope(
       OldVar->isPreviousDeclInSameBlockScope());
   NewVar->setAccess(OldVar->getAccess());
 
   if (!OldVar->isStaticDataMember()) {
     if (OldVar->isUsed(false))
-      NewVar->setIsUsed();
-    NewVar->setReferenced(OldVar->isReferenced());
+      setDeclIsUsed(NewVar);
+    setDeclReferenced(static_cast<Decl *>(NewVar), OldVar->isReferenced());
   }
 
   InstantiateAttrs(TemplateArgs, OldVar, NewVar, LateAttrs, StartingScope);
 
   LookupResult Previous(
       *this, NewVar->getDeclName(), NewVar->getLocation(),
       NewVar->isLocalExternDecl() ? Sema::LookupRedeclarationWithLinkage
                                   : Sema::LookupOrdinaryName,

clang/lib/Serialization/ASTReaderDecl.cpp

if (unsigned SubmoduleID = readSubmoduleID()) {
     switch (ModuleOwnership) {
     case Decl::ModuleOwnershipKind::Visible:
       ModuleOwnership = Decl::ModuleOwnershipKind::VisibleWhenImported;
       break;
     case Decl::ModuleOwnershipKind::Unowned:
     case Decl::ModuleOwnershipKind::VisibleWhenImported:
     case Decl::ModuleOwnershipKind::ReachableWhenImported:
     case Decl::ModuleOwnershipKind::ModulePrivate:
+    case Decl::ModuleOwnershipKind::ModuleDiscardable:
       break;
     }
 
     D->setModuleOwnershipKind(ModuleOwnership);
     // Store the owning submodule ID in the declaration.
     D->setOwningModuleID(SubmoduleID);
 
-    if (ModulePrivate) {
-      // Module-private declarations are never visible, so there is no work to
-      // do.
+    if (ModulePrivate ||
+        ModuleOwnership == Decl::ModuleOwnershipKind::ModuleDiscardable) {
+      // Module-private and unreachable declarations are never visible, so
+      // there is no work to do.
     } else if (Reader.getContext().getLangOpts().ModulesLocalVisibility) {
       // If local visibility is being tracked, this declaration will become
       // hidden and visible as the owning module does.
     } else if (Module *Owner = Reader.getSubmodule(SubmoduleID)) {
       // Mark the declaration as visible when its owning module becomes visible.
       if (Owner->NameVisibility == Module::AllVisible)
         D->setVisibleDespiteOwningModule();
       else
         Reader.HiddenNamesMap[Owner].push_back(D);
     }
+  } else if (ModuleOwnership == Decl::ModuleOwnershipKind::ModuleDiscardable) {
+    D->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModuleDiscardable);
   } else if (ModulePrivate) {
     D->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModulePrivate);
   }
 }
 
 void ASTDeclReader::VisitPragmaCommentDecl(PragmaCommentDecl *D) {
   VisitDecl(D);
   D->setLocation(readSourceLocation());
   D->CommentKind = (PragmaMSCommentKind)Record.readInt();
   std::string Arg = readString();

clang/test/AST/ast-dump-decl.c

 // Test without serialization:
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-strict-prototypes -ast-dump -ast-dump-filter Test %s \
-// RUN: | FileCheck --strict-whitespace %s
+// RUN: | FileCheck --strict-whitespace %s -check-prefix CHECK-NOMODULE
 //
 // Test with serialization:
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-strict-prototypes -emit-pch -o %t %s
 // RUN: %clang_cc1 -x c -triple x86_64-unknown-unknown -Wno-strict-prototypes -include-pch %t \
 // RUN: -ast-dump-all -ast-dump-filter Test /dev/null \
 // RUN: | sed -e "s/ <undeserialized declarations>//" -e "s/ imported//" \
-// RUN: | FileCheck --strict-whitespace %s
+// RUN: | FileCheck --strict-whitespace %s -check-prefix CHECK-NOMODULE
 //
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-strict-prototypes -ast-dump %s \
-// RUN: | FileCheck -check-prefix CHECK-TU --strict-whitespace %s
+// RUN: | FileCheck -check-prefix CHECK-TU --strict-whitespace %s -check-prefix CHECK-NOMODULE
 //
 // RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fmodule-name=X \
 // RUN: -triple x86_64-unknown-unknown -Wno-strict-prototypes -fmodule-map-file=%S/Inputs/module.modulemap \
 // RUN: -ast-dump -ast-dump-filter Test %s -DMODULES \
 // RUN: | FileCheck -check-prefix CHECK -check-prefix CHECK-MODULES --strict-whitespace %s
 
 int TestLocation;
 // CHECK: VarDecl 0x{{[^ ]*}} <{{.*}}:[[@LINE-1]]:1, col:5> col:5 TestLocation

 // CHECK:      FieldDecl{{.*}} TestFieldDecl 'int'
 // CHECK:      FieldDecl{{.*}} TestFieldDeclWidth 'int'
 // CHECK-NEXT:   ConstantExpr
 // CHECK-NEXT:     value: Int 1
 // CHECK-NEXT:       IntegerLiteral
 // CHECK-MODULE:      FieldDecl{{.*}} TestFieldDeclPrivate 'int' __module_private__
 
 int TestVarDecl;
-// CHECK:      VarDecl{{.*}} TestVarDecl 'int'
+// CHECK-NOMODULE:      VarDecl{{.*}} TestVarDecl 'int'
+// CHECK-MODULE:      VarDecl{{.*}} TestVarDecl 'int' VisibleWhenImported
 
 extern int TestVarDeclSC;
-// CHECK:      VarDecl{{.*}} TestVarDeclSC 'int' extern
+// CHECK-NOMODULE:      VarDecl{{.*}} TestVarDeclSC 'int' extern
+// CHECK-MODULE:      VarDecl{{.*}} TestVarDeclSC 'int' extern VisibleWhenImported
 
 __thread int TestVarDeclThread;
-// CHECK:      VarDecl{{.*}} TestVarDeclThread 'int' tls{{$}}
+// CHECK-NOMODULE:      VarDecl{{.*}} TestVarDeclThread 'int' tls{{$}}
+// CHECK-MODULE:      VarDecl{{.*}} TestVarDeclThread 'int' tls{{$}} VisibleWhenImported
 
 __module_private__ int TestVarDeclPrivate;
 // CHECK-MODULE:      VarDecl{{.*}} TestVarDeclPrivate 'int' __module_private__
 
 int TestVarDeclInit = 0;
 // CHECK:      VarDecl{{.*}} TestVarDeclInit 'int'
 // CHECK-NEXT:   IntegerLiteral
 
 void testParmVarDecl(int TestParmVarDecl);
 // CHECK: ParmVarDecl{{.*}} TestParmVarDecl 'int'
 
 #ifdef MODULES
 #pragma clang module end
 #endif
 

clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp

 int not_exported_private;
 static int internal_private;
 #else
 
 #ifdef IMPLEMENTATION
 module;
 #endif
 
 void test_early() {
-  in_header = 1; // expected-error {{missing '#include "foo.h"'; 'in_header' must be declared before it is used}}
-  // expected-note@* {{not visible}}
-
+  in_header = 1;              // expected-error {{missing '#include "foo.h"'; 'in_header' must be declared before it is used}}
+                              // expected-note@foo.h:3 {{declaration here is not visible}}
   global_module_fragment = 1; // expected-error {{missing '#include'; 'global_module_fragment' must be declared before it is used}}
                               // [e-mail]:16 {{not visible}}
 
   exported = 1; // expected-error {{must be imported from module 'A'}}
                 // [e-mail]:18 {{declaration here is not visible}}
 
   not_exported = 1; // expected-error {{declaration of 'not_exported' must be imported from module 'A' before it is required}}
                     // [e-mail]:19 {{declaration here is not visible}}

 
 #ifdef IMPLEMENTATION
 module A;
 #else
 import A;
 #endif
 
 void test_late() {
-  in_header = 1; // expected-error {{missing '#include "foo.h"'; 'in_header' must be declared before it is used}}
-  // expected-note@* {{not visible}}
-
+  in_header = 1;              // expected-error {{missing '#include "foo.h"'; 'in_header' must be declared before it is used}}
+                              // expected-note@foo.h:3 {{declaration here is not visible}}
   global_module_fragment = 1; // expected-error {{missing '#include'; 'global_module_fragment' must be declared before it is used}}
   // [e-mail]:16 {{not visible}}
 
   exported = 1;
 
   not_exported = 1;
 #ifndef IMPLEMENTATION
   // expected-error@-2 {{declaration of 'not_exported' must be imported from module 'A' before it is required}}

clang/test/CXX/class/class.friend/p7-cxx20.cpp

 module;
 #include "header.h"
 
 export module M;
 
 class Z {
   friend void z(){};
 };
-// CHECK-MOD: |-CXXRecordDecl {{.*}} <./header.h:2:1, line:4:1> line:2:7 in M.<global> hidden class A definition
-// CHECK-MOD: | |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M.<global> hidden implicit class A
-// CHECK-MOD-NEXT: | `-FriendDecl {{.*}} <line:3:3, col:19> col:15 in M.<global>
-// CHECK-MOD-NEXT: |   `-FunctionDecl {{.*}} parent {{.*}} <col:3, col:19> col:15 in M.<global> hidden a 'void ()' implicit-inline
+// CHECK-MOD: |-CXXRecordDecl {{.*}} <./header.h:2:1, line:4:1> line:2:7 in M.<global> hidden class A{{( ModuleDiscardable)?}} definition
+// CHECK-MOD: | |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M.<global> hidden implicit class A{{( ModuleDiscardable)?}}
+// CHECK-MOD-NEXT: | `-FriendDecl {{.*}} <line:3:3, col:19> col:15 in M.<global>{{( ModuleDiscardable)?}}
+// CHECK-MOD-NEXT: |   `-FunctionDecl {{.*}} parent {{.*}} <col:3, col:19> col:15 in M.<global> hidden a 'void ()'{{( ModuleDiscardable)?}} implicit-inline
 
 // CHECK-MOD: `-CXXRecordDecl {{.*}} <module.cpp:6:1, line:8:1> line:6:7 in M hidden class Z{{( ReachableWhenImported)?}} definition
 // CHECK-MOD: |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M hidden implicit class Z{{( ReachableWhenImported)?}}
 // CHECK-MOD-NEXT: `-FriendDecl {{.*}} <line:7:3, col:19> col:15 in M{{( ReachableWhenImported)?}}
 // CHECK-MOD-NEXT: `-FunctionDecl {{.*}} parent {{.*}} <col:3, col:19> col:15 in M hidden z 'void ()'{{( ReachableWhenImported)?}}

clang/test/CXX/class/class.mfct/p1-cxx20.cpp

 #include "header.h"
 
 export module M;
 
 class Z {
   void z(){};
 };
 
-// CHECK-MOD: |-CXXRecordDecl {{.*}} <./header.h:2:1, line:4:1> line:2:7 in M.<global> hidden class A definition
-// CHECK-MOD: | |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M.<global> hidden implicit class A
-// CHECK-MOD-NEXT: | `-CXXMethodDecl {{.*}} <line:3:3, col:12> col:8 in M.<global> hidden a 'void ()' implicit-inline
+// CHECK-MOD: |-CXXRecordDecl {{.*}} <./header.h:2:1, line:4:1> line:2:7 in M.<global> hidden class A{{( ModuleDiscardable)?}} definition
+// CHECK-MOD: | |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M.<global> hidden implicit class A{{( ModuleDiscardable)?}}
+// CHECK-MOD-NEXT: | `-CXXMethodDecl {{.*}} <line:3:3, col:12> col:8 in M.<global> hidden a 'void ()'{{( ModuleDiscardable)?}} implicit-inline
 
 // CHECK-MOD: `-CXXRecordDecl {{.*}} <module.cpp:6:1, line:8:1> line:6:7 in M hidden class Z{{( ReachableWhenImported)?}} definition
 // CHECK-MOD: |-CXXRecordDecl {{.*}} <col:1, col:7> col:7 in M hidden implicit class Z{{( ReachableWhenImported)?}}
 // CHECK-MOD-NEXT: `-CXXMethodDecl {{.*}} <line:7:3, col:12> col:8 in M hidden z 'void ()'{{( ReachableWhenImported)?}}

clang/test/CXX/module/module.global.frag/cxx20-10-4-ex2.cpp

+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 std-10-4-ex2-interface.cpp \
+// RUN:  -emit-module-interface -o M.pcm -Wno-unused-value
+// RUN: %clang_cc1 -std=c++20 std-10-4-ex2-implementation.cpp \
+// RUN:  -fmodule-file=M.pcm -fsyntax-only -verify
+//--- std-10-4-ex2.h
+
+namespace N {
+struct X {};
+int d();
+int e();
+inline int f(X, int = d()) { return e(); }
+int g(X);
+int h(X);
+} // namespace N
+
+//--- std-10-4-ex2-interface.cpp
+
+module;
+
+#include "std-10-4-ex2.h"
+
+export module M;
+
+template <typename T> int use_f() {
+  N::X x;           // N::X, N, and  ::  are decl-reachable from use_f
+  return f(x, 123); // N::f is decl-reachable from use_f,
+                    // N::e is indirectly decl-reachable from use_f
+                    //   because it is decl-reachable from N::f, and
+                    // N::d is decl-reachable from use_f
+                    //   because it is decl-reachable from N::f
+                    //   even though it is not used in this call
+}
+
+template <typename T> int use_g() {
+  N::X x;             // N::X, N, and :: are decl-reachable from use_g
+  return g((T(), x)); // N::g is not decl-reachable from use_g
+}
+
+template <typename T> int use_h() {
+  N::X x;             // N::X, N, and :: are decl-reachable from use_h
+  return h((T(), x)); // N::h is not decl-reachable from use_h, but
+                      // N::h is decl-reachable from use_h<int>
+}
+
+int k = use_h<int>();
+// use_h<int> is decl-reachable from k, so
+// N::h is decl-reachable from k
+
+//--- std-10-4-ex2-implementation.cpp
+
+module M;
+
+int a = use_f<int>();
+int b = use_g<int>(); // [e-mail]:20 {{use of undeclared identifier 'g'}}
+                      // expected-note@-1 {{in instantiation of function template specialization 'use_g<int>' requested here}}
+int c = use_h<int>();

clang/test/CXX/module/module.global.frag/p3-p4.cpp

+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 M.cpp -emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -std=c++20 M.cpp -ast-dump | FileCheck --match-full-lines %s
+// RUN: %clang_cc1 -std=c++20 M-impl.cpp -fmodule-file=M.pcm -fsyntax-only -verify
+
+//--- p3-p4.h
+int f();
+int g();
+int h();
+int j();
+typedef int GMFInt;
+
+//--- M.cpp
+module;
+#include "p3-p4.h"
+
+export module M;
+
+export int use_f() { return f(); }
+
+export using ::h;
+
+namespace N {
+export using ::j;
+}
+
+GMFInt k(int);
+
+// CHECK: |-FunctionDecl {{.*}} <./p3-p4.h:1:1, col:7> col:5 in M.<global> hidden used f 'int ()' ReachableWhenImported
+// CHECK: |-FunctionDecl {{.*}} <line:2:1, col:7> col:5 in M.<global> hidden g 'int ()' ModuleDiscardable
+// CHECK: |-FunctionDecl {{.*}} <line:3:1, col:7> col:5 in M.<global> hidden h 'int ()' ReachableWhenImported
+// CHECK: |-FunctionDecl {{.*}} <line:4:1, col:7> col:5 in M.<global> hidden j 'int ()' ReachableWhenImported
+// CHECK: |-TypedefDecl {{.*}} <line:5:1, col:13> col:13 in M.<global> hidden referenced GMFInt 'int' ReachableWhenImported
+
+// CHECK: |-ExportDecl {{.*}} <M.cpp:6:1, col:34> col:1 in M
+// CHECK-NEXT: | `-FunctionDecl {{.*}} <col:8, col:34> col:12 in M hidden use_f 'int ()' VisibleWhenImported
+
+// CHECK: |-ExportDecl {{.*}} <line:8:1, col:16> col:1 in M
+// CHECK-NEXT: | |-UsingDecl {{.*}} <col:8, col:16> col:16 in M hidden ::h VisibleWhenImported
+// CHECK-NEXT: | `-UsingShadowDecl {{.*}} <col:16> col:16 in M hidden implicit Function {{.*}} 'h' 'int ()' VisibleWhenImported
+
+// CHECK: |-NamespaceDecl {{.*}} <line:10:1, line:12:1> line:10:11 in M hidden N
+// CHECK-NEXT: | `-ExportDecl {{.*}} <line:11:1, col:16> col:1 in M
+// CHECK-NEXT: |   |-UsingDecl {{.*}} <col:8, col:16> col:16 in M hidden ::j VisibleWhenImported
+// CHECK-NEXT: |   `-UsingShadowDecl {{.*}} <col:16> col:16 in M hidden implicit Function {{.*}} 'j' 'int ()' VisibleWhenImported
+
+//--- M-impl.cpp
+import M;
+int a = j();    // expected-error {{missing '#include'; 'j' must be declared before it is used}}
+                // expected-note@p3-p4.h:4 {{declaration here is not visible}}
+int b = N::j(); // should be OK, UsingShadowDecl is visible

clang/test/Modules/cxx-templates.cpp

 // CHECK-NAMESPACE-N-NEXT: |-FunctionTemplate {{.*}} 'f'
 // CHECK-NAMESPACE-N-NEXT: |-FunctionTemplate {{.*}} 'f'
 // CHECK-NAMESPACE-N-NEXT: |-FunctionTemplate {{.*}} 'f'
 // CHECK-NAMESPACE-N-NEXT: `-FunctionTemplate {{.*}} 'f'
 
 // CHECK-DUMP:      ClassTemplateDecl {{.*}} <{{.*[/\\]}}cxx-templates-common.h:1:1, {{.*}}>  col:{{.*}} in cxx_templates_common SomeTemplate
 // CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
 // CHECK-DUMP-NEXT:     TemplateArgument type 'char[2]'
-// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition
+// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} SomeTemplate Visible definition
 // CHECK-DUMP-NEXT:     DefinitionData
 // CHECK-DUMP-NEXT:       DefaultConstructor
 // CHECK-DUMP-NEXT:       CopyConstructor
 // CHECK-DUMP-NEXT:       MoveConstructor
 // CHECK-DUMP-NEXT:       CopyAssignment
 // CHECK-DUMP-NEXT:       MoveAssignment
 // CHECK-DUMP-NEXT:       Destructor
 // CHECK-DUMP-NEXT:     TemplateArgument type 'char[2]'
 // CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
 // CHECK-DUMP-NEXT:     TemplateArgument type 'char[1]'
-// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition
+// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} SomeTemplate Visible definition
 // CHECK-DUMP-NEXT:     DefinitionData
 // CHECK-DUMP-NEXT:       DefaultConstructor
 // CHECK-DUMP-NEXT:       CopyConstructor
 // CHECK-DUMP-NEXT:       MoveConstructor
 // CHECK-DUMP-NEXT:       CopyAssignment
 // CHECK-DUMP-NEXT:       MoveAssignment
 // CHECK-DUMP-NEXT:       Destructor
 // CHECK-DUMP-NEXT:     TemplateArgument type 'char[1]'

lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py

class TestTemplateWithSameArg(TestBase):
     @add_test_categories(["gmodules"])
     def test_duplicate_decls(self):
         lldbutil.run_to_source_breakpoint(self, "Break here", self.main_source_file)
 
         self.expect_expr("(intptr_t)&FromMod1 + (intptr_t)&FromMod2")
 
         # Make sure we only have a single 'Member' decl on the AST
         self.filecheck("target module dump ast", __file__)
-# CHECK:      ClassTemplateSpecializationDecl {{.*}} imported in Module2 struct ClassInMod3 definition
-# CHECK-NEXT: |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial
+# CHECK:      ClassTemplateSpecializationDecl {{.*}} imported in Module2 struct ClassInMod3 Visible definition
+# CHECK-NEXT: |-DefinitionData Visible pass_in_registers aggregate standard_layout trivially_copyable pod trivial
 # CHECK-NEXT: | |-DefaultConstructor exists trivial needs_implicit
 # CHECK-NEXT: | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
 # CHECK-NEXT: | |-MoveConstructor exists simple trivial needs_implicit
 # CHECK-NEXT: | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
 # CHECK-NEXT: | |-MoveAssignment exists simple trivial needs_implicit
 # CHECK-NEXT: | `-Destructor simple irrelevant trivial needs_implicit
 # CHECK-NEXT: |-TemplateArgument type 'int'
 # CHECK-NEXT: | `-BuiltinType {{.*}} 'int'
 # CHECK-NEXT: `-FieldDecl {{.*}} imported in Module2 Member 'int'