Refactor CanForwardDeclareType for better sugar tolerance

These changes have no effect on the current test suite, but there are
obvious weaknesses when sugared types are taken into account.

The way CanForwardDeclareType asks if the parent is-a particular AST
node type is generally unsound in the presence of sugar, such as
ElaboratedType, which may show up above any Type in the tree.

An example:

  class C {};
  typedef C FooBar;

produces:

  `-TypedefDecl 0x559e505c6e30
    `-ElaboratedType 0x559e505c6df0 'C' sugar
      `-RecordType 0x559e505c6cc0 'C'
        `-CXXRecord 0x559e505c6c30 'C'

If we were to call CanForwardDeclare for the type C, it would not
classify as part of a TypedefDecl, because its parent is-a
ElaboratedType.

Therefore, delay any parent type checking until we've rewinded the
current AST node using MostElaboratedAncestor (there's a case to be made
that we should add a more general MostSugaredAncestor to walk _up_ the
tree until a desugared node is reached, much the same way as Desugar
will walk _down_ the tree. But that's good idea for a separate project.)

This was inspired by Clang 15f3cd6bfc670ba6106184a903eb04be059e5977,
which wraps the majority of Type nodes in an additional ElaboratedType
node.

1 files changed, 27 insertions, 24 deletions

diff --git a/iwyu.cc b/iwyu.cc
index 8c80174..d395247 100644
--- a/iwyu.cc
+++ b/iwyu.cc
@@ -210,6 +210,7 @@ using clang::TemplateSpecializationType;
 using clang::TemplateSpecializationTypeLoc;
 using clang::TranslationUnitDecl;
 using clang::Type;
+using clang::TypeDecl;
 using clang::TypeLoc;
 using clang::TypedefDecl;
 using clang::TypedefNameDecl;
@@ -2569,20 +2570,6 @@ class IwyuBaseAstVisitor : public BaseAstVisitor<Derived> {
     // If we're in a forward-declare context, well then, there you have it.
     if (ast_node->in_forward_declare_context())
       return true;
-    // If we're in a typedef, we don't want to forward-declare even if
-    // we're a pointer.  ('typedef Foo* Bar; Bar x; x->a' needs full
-    // type of Foo.)
-    if (ast_node->ParentIsA<TypedefNameDecl>())
-      return false;
-
-    // If we ourselves are a forward-decl -- that is, we're the type
-    // component of a forward-declaration (which would be our parent
-    // AST node) -- then we're forward-declarable by definition.
-    if (const TagDecl* parent
-        = current_ast_node()->template GetParentAs<TagDecl>()) {
-      if (IsForwardDecl(parent))
-        return true;
-    }
 
     // Another place we disregard what the language allows: if we're
     // a dependent type, in theory we can be forward-declared.  But
@@ -2595,16 +2582,15 @@ class IwyuBaseAstVisitor : public BaseAstVisitor<Derived> {
     // Read past elaborations like 'class' keyword or namespaces.
     ast_node = MostElaboratedAncestor(ast_node);
 
-    // Now there are two options: either we have a type or we have a declaration
-    // involving a type.
+    // Now there are two options: either we are part of a type or we are part of
+    // a declaration involving a type.
     const Type* parent_type = ast_node->GetParentAs<Type>();
     if (parent_type == nullptr) {
-      // Since it's not a type, it must be a decl.
-      // Our target here is record members, all of which derive from ValueDecl.
-      if (const ValueDecl *decl = ast_node->GetParentAs<ValueDecl>()) {
+      // It's not a type; analyze different kinds of declarations.
+      if (const auto *decl = ast_node->GetParentAs<ValueDecl>()) {
         // We can shortcircuit static data member declarations immediately,
         // they can always be forward-declared.
-        if (const VarDecl *var_decl = DynCastFrom(decl)) {
+        if (const auto *var_decl = dyn_cast<VarDecl>(decl)) {
           if (!var_decl->isThisDeclarationADefinition() &&
               var_decl->isStaticDataMember()) {
             return true;
@@ -2612,10 +2598,27 @@ class IwyuBaseAstVisitor : public BaseAstVisitor<Derived> {
         }
 
         parent_type = GetTypeOf(decl);
-      } else if (IsElaboratedTypeSpecifier(ast_node)) {
-        // If it's not a ValueDecl, it must be a type decl. Elaborated types in
-        // type decls are automatically forward-declarable.
-        return true;
+      } else if (const auto *decl = ast_node->GetParentAs<TypeDecl>()) {
+        // Elaborated types in type decls are always forward-declarable
+        // (and usually count as forward declarations themselves).
+        if (IsElaboratedTypeSpecifier(ast_node)) {
+          return true;
+        }
+
+        // If we ourselves are a forward-decl -- that is, we're the type
+        // component of a forward-declaration (which would be our parent
+        // AST node) -- then we're forward-declarable by definition.
+        if (const auto* parent_decl = ast_node->GetParentAs<TagDecl>()) {
+          if (IsForwardDecl(parent_decl))
+            return true;
+        }
+
+        // If we're part of a typedef declaration, we don't want to forward-
+        // declare even if we're a pointer ('typedef Foo* Bar; Bar x; x->a'
+        // needs full type of Foo.)
+        if (ast_node->ParentIsA<TypedefNameDecl>()) {
+          return false;
+        }
       }
     }