QDoc: Remove `ClangVisitor::m_friendDecl`

QDoc parses C++ code that is part of a documentation project through the
use of a Clang-based parser, using the extracted information both to
find user-defined documentation and to check that it has certain
qualities.

QDoc extracts that kind of information by traversing the AST produced by
calls to Libclang, Clang's C API, and slowly lowering that AST into a
custom AST made of `Node`s.

This traversal is generally handled by `ClangVisitor` as called by
`ClangCodeParser`, the top level interface for parsing C++ source and
header files.

When a function/method is defined as `friend` of a certain element, QDoc
extracts that information and specially marks the corresponding `Node` so
that it can be categorized and highlighted in a certain way when shown
to the user.

In the AST that Clang produces, such an element that is marked as a
`friend` produces an AST node, `FriendDeclaration`, or a cursor of kind
`CXCursor_FriendDecl`, whose child is the marked element.

`ClangVisitor` is built so that, while traversing the AST, no context
is preversed between the inspection of each node.
Furthermore, the traversal is one-directional, so that no information
found in child elements is bubbled up to a previous step of
the traversal.
With those self-imposed restrictions in mind, to handle the information
of a declaration being a `friend`, a boolean member
`ClangVisitor::m_friendDecl` is used.
The member is set to `true` when a `CXCursor_FriendDecl` is found,
inspected at a later point in the traversal of the cursor children,
and finally set back to `false` when all the children of the cursor
have been visited.

The `m_friendDecl` member was then used while processing other elements
of the AST to discern whether the element that was being processed
should be marked as a `friend` or not.

Due to the recursion of the AST, the place where the boolean flag was
set and the place where it was actually used could be quite apart,
sometimes down many levels of a call stack, reducing the locality and
scrutability of the downstream code.

Due to the instance-scope of the member, extraneous children of a
`CXCursor_FriendDecl` could be evaluated as `friend`s even when that
could not have a valid semantic.
Similarly, due to the mutability of the member and its excessive scope,
care was required to ensure that the flag was reset at the correct point
during the traversal, extending the mental load required to understand
the code and the possible classes of bugs.

To simplify the code, `m_friendDecl` is now removed, in favor of
in-place, AST-based checks to evaluate whether an element is a `friend`
or not.

The member variable was removed from the instance, along with its single
write access in `ClangVisitor::visitHeader`.

Additionally, its single read access in `ClangVisitor::processFunction`
was modified to retrieve the `friend` information through Clang's APIs.
Specifically, the C++ API from Clang, which has been introduced by
recent changes in QDoc that allows it to be used along with Clang's C
API, is used to obtain such information, through the
`Decl::getFriendObjectKind`, as it is more expressive and more direct
than the required C API's calls.

Certain unnecessary comments in the touched code were removed as they
were providing no useful information.

The change does not impact the produced documentation and is expected to
be semantically equivalent to the behavior of the previous code.

Change-Id: Ifb453ac6d29fa9e2db90f311a1656c95b9dcd712
Reviewed-by: Paul Wicking <paul.wicking@qt.io>

1 files changed, 3 insertions, 5 deletions

diff --git a/src/qdoc/clangcodeparser.cpp b/src/qdoc/clangcodeparser.cpp
index 163ce20f7..d2338ee32 100644
--- a/src/qdoc/clangcodeparser.cpp
+++ b/src/qdoc/clangcodeparser.cpp
@@ -27,6 +27,7 @@
 #include <clang-c/Index.h>
 
 #include <clang/AST/Decl.h>
+#include <clang/AST/DeclFriend.h>
 #include <clang/AST/DeclTemplate.h>
 #include <clang/AST/Expr.h>
 #include <clang/AST/Type.h>
@@ -489,7 +490,6 @@ private:
 
     QDocDatabase *qdb_;
     Aggregate *parent_;
-    bool m_friendDecl { false };             // true if currently visiting a friend declaration
     const QMultiHash<QString, QString> allHeaders_;
     QHash<CXFile, bool> isInterestingCache_; // doing a canonicalFilePath is slow, so keep a cache.
 
@@ -762,8 +762,6 @@ CXChildVisitResult ClangVisitor::visitHeader(CXCursor cursor, CXSourceLocation l
     }
 #if CINDEX_VERSION >= 36
     case CXCursor_FriendDecl: {
-        QScopedValueRollback<bool> setFriend(m_friendDecl, true);
-        // Visit the friend functions
         return visitChildren(cursor);
     }
 #endif
@@ -1038,8 +1036,8 @@ void ClangVisitor::processFunction(FunctionNode *fn, CXCursor cursor)
     if (clang_isFunctionTypeVariadic(funcType))
         parameters.append(QStringLiteral("..."));
     readParameterNamesAndAttributes(fn, cursor);
-    // Friend functions are not members
-    if (m_friendDecl)
+
+    if (declaration->getFriendObjectKind() != clang::Decl::FOK_None)
         fn->setRelatedNonmember(true);
 }