The way in which you're checking for the problematic cases is unnecessarily expensive. Instead of performing a separate AST traversal, please detect whether you should be producing the warning directly when forming the problematic expressions. (For example, you could store a list of pending "dereference of noderef" expressions in the ExpressionEvaluationContextRecord, remove items from the list if you find they're the operand of a unary address-of operator, and diagnose any remaining items once you've left the evaluation context.)

Looks good with one cleanup.

Thanks!

Can we avoid the duplication by putting this check in Sema::ParsedFreeStandingDeclSpec instead?

In D48322#1150316, @erik.pilkington wrote:

Is this what you were concerned about?

Looks fine, but please put the test case somewhere more appropriate (under SemaCXX, for instance).

Looks right to me (other than the missing constexpr in C++14 onwards). Though this is subtle enough that I suspect the only way to know for sure is to try it.

r336013

Thanks, I think this test needs a target triple to avoid the funny msvc
enum rules. I'm working on it.

Hmm, so this will mean that we can have internal linkage declarations marked Used for which there is no definition, and we need to not warn on that.

This is not specific to the ASTImporter; any change to the AST after a call to getParents would have similar problems. Generally, responsibility for dealing with this must lie with the consumer of the parent map, not with the ASTContext, since the ASTContext generally doesn't even know when the AST gets mutated.

I'm not 100% thrilled that we're emitting two warnings about the same thing for slightly different reasons; alternatives welcome. :)