It was very good to separate this out, thanks. Can you can do some TMP performance testing, to verify the impacts are negligible before taking resugaring into consideration, to allay potential concerns?

The second paragraph is talking about 'Canonical nodes', not 'Canonical types'.

A canonical node is a type node for which 'isSugared' method returns false.

This part of the description is confusing:

This corrects a genuine deficiency in the AST, and the patch LGTM. Can we knock this off Matheus' stack?

In D117390#3246799, @kimgr wrote:

@daverec I don't have commit access, could you help me land this?

I've rebased on main without conflicts, and ninja check-clang ran successfully after rebase.

Let me know if there's anything else I can do as far as prep work.

Looks good, thanks for fixing this!

There are already two way more sophisticated (forgive me my bias) implementations in Clang that are for checking if two statements or decls are the same.

1. ODRHash, used in modules to discover ODR violations
2. ASTStructuralEquivalenceContext, used in ASTImporter to discover if two AST nodes are the same or not (as a side effect we diagnose ODR violations as well).

It is not the first time, when such a similarity check is needed (see https://reviews.llvm.org/D75041). Of course reusing the before mentioned components would require some architectural changes, but it might be beneficial.

throughUsingDecl seems like a good solution, though I defer to regular ASTMatchers users.

A couple thoughts/cases to consider...

Looks great, thanks for identifying the need and banging this out so quickly.
Hope you had some time to enjoy the holiday with your family!
Dave

Looks good, a few notes.

Was this performance hit when using the static analyzer? A quick search suggests isAnyDestructorNoReturn() is only called within the analyzer, whereas comparable CXXRecordDecl methods whose results are stored (hasIrrelevantDestructor() etc.) seem to be called somewhere by Sema.

Looks good, thanks for doing this!

Sorry for the delay.
Richard should probably weigh in before absolutely committing to

1. BaseUsingDecl/UsingEnumDecl/UsingDecl (as implemented here) vs. single UsingDecl (Nathan's original approach) and
2. Renaming getUsingDecl() to getIntroducer() (if it is to return a BaseUsingDecl).