Richard
Richard

Reputation: 61289

clang-tidy: Warn when a container holds a particular class

Using LLVM-15's clang-tidy linter, I'd like to enforce the use of a typedef. Instances of

std::unordered_set<my::namespace1::Class>

used as a variable declaration, function return, or in a using/typedef, should instead be written as

MyClassSet

where

using MyClassSet = std::unordered_set<my::namespace1::Class>

I've tried, eg,

  const auto matcher =
      varDecl(allOf(
                  cxxRecordDecl(matchesName("std::unordered_set")),
                  classTemplateSpecializationDecl(hasTemplateArgument(
                      0,
                      templateArgument(refersToType(hasDeclaration(
                          cxxRecordDecl(matchesName("my::namespace1::Class")))))))))
          .bind("uset");

but this doesn't yield any matches and it's unclear to me whether it would actually avoid matching correct uses of the typedef.

Any help figuring out the right matcher would be appreciated!

Upvotes: 2

Views: 181

Answers (1)

Scott McPeak
Scott McPeak

Reputation: 12749

Quick fix

The attempt in the question almost works for a subset of cases. It cannot match anything because varDecl(allOf(cxxRecordDecl(...))) requires a matching entity to be both a VarDecl and CXXRecordDecl, which is not possible (they are distinct subclasses of NamedDecl).

The fix is to insert hasType before allOf to constrain the type of the declared thing, rather than the declared thing itself. With that one change, the query now reports variable and parameter declarations, but not return types or typedef/using aliases, and does not whitelist the MyClassSet alias.

Handling those other cases requires slightly adjusting the details of how the type is recognized, as shown below.

Matcher satisfying the specification

Here is a clang-query command that reports any variable, parameter, field, or typedef/using alias where the type is std::unordered_set<my::namespace1::Class>, so long as that type is expressed as a template specialization, as opposed to using a particular whitelisted using alias. It is written as a shell script with two preliminary variable assignments for improved readability:

# Common fragment for matching the specialization of interest.
typeMatcher='
  hasDeclaration(
    classTemplateSpecializationDecl(
      hasName("::std::unordered_set"),
      hasTemplateArgument(0,
        refersToType(
          hasDeclaration(
            cxxRecordDecl(
              hasName("::my::namespace1::Class")
            )
          )
        )
      )
    )
  )
'

# Query covering the syntactic cases of interest.
#
# In this query, the comments are ignored because clang-query (not the
# shell) recognizes and discards them.
query="m
  decl(
    anyOf(
      # Declarator declarations include variables and fields.
      declaratorDecl(
        anyOf(
          # Report when the variable (etc.) has the exact type.
          hasType(
            $typeMatcher
          ),

          # Also report references to that type.
          hasType(
            referenceType(
              pointee(
                $typeMatcher
              )
            )
          ),

          # And pointers to the type.
          hasType(
            pointerType(
              pointee(
                $typeMatcher
              )
            )
          )
        )
      ),

      # Look at function declarations to see return types.
      functionDecl(
        returns(
          $typeMatcher
        )
      ),

      # This matches both typedef and 'using A = B' aliases.
      typedefNameDecl(
        hasType(
          $typeMatcher
        ),

        # Whitelist the one allowed 'using' alias.
        unless(
          # These backslashes are shell escape syntax, not Clang AST
          # matcher syntax.
          hasName(\"MyClassSet\")
        )
      )
    )
  )"

# The 'IgnoreUnlessSpelledInSource' flag suppresses reports that would
# otherwise arise from within the instantiated template.
clang-query \
  -c='set traversal IgnoreUnlessSpelledInSource' \
  -c="$query" \
  test.cc --

Test case:

// test.cc
// Test clang-query finding the declaration of a variable that is a
// specialization (instantiation) of std::unordered_set with a
// particular class argument.

#include <unordered_set>               // std::unordered_set

namespace my {
  namespace namespace1 {
    // The class that should not be used with std::unordered_set, except
    // via a particular whitelisted 'using' alias.
    class Class {
    public:
      Class() {}
    };
  }
}

// Trivial specialization of std::hash to satisfy the requirements for
// std::unordered_set<my::namespace1::Class>.
template <>
struct std::hash<my::namespace1::Class> {
  std::size_t operator() (my::namespace1::Class const &obj) const noexcept
  {
    return 1;
  }
};


// ---- Things to report ----

// The goal is to report this variable declaration.
std::unordered_set<my::namespace1::Class> instanceToReport1;

// Also report when the namespace is not given explicitly.
using namespace my::namespace1;
std::unordered_set<Class> instanceToReport2;

// We presumably also want to report class fields.
class ClassWithField {
public:
  std::unordered_set<my::namespace1::Class> fieldToReport;
};

// Report parameters, including pointers and references.
void reportParameter1(std::unordered_set<Class> p);
void reportParameter2(std::unordered_set<Class> &rp);
void reportParameter3(std::unordered_set<Class> *pp);

// Report when used as function return type.
std::unordered_set<Class> reportReturnType(int);

// Report creating a 'typedef' or 'using' alias other than the
// whitelisted one.
typedef std::unordered_set<Class> UnallowedTypedefAlias;
using UnallowedUsingAlias = std::unordered_set<Class>;


// ---- Things to NOT report ----

// Not reported because it does not use the container.
my::namespace1::Class x;

// A different class that *is* ok to use with std::unordered_set
// directly.
class SomeOtherClass {
public:
  SomeOtherClass() {}
};

template <>
struct std::hash<SomeOtherClass> {
  std::size_t operator() (SomeOtherClass const &obj) const noexcept
  {
    return 1;
  }
};

// Not reported because the argument is not "my::namespace1::Class".
std::unordered_set<SomeOtherClass> someOtherInstance;

// Not reported because it is whitelisted.
using MyClassSet = std::unordered_set<my::namespace1::Class>;

// Not reported because it uses the typedef.
MyClassSet allowedInstance;

template <class T>
class SomeOtherContainer {};

// Not reported because this is a different container than
// std::unordered_set.
SomeOtherContainer<my::namespace1::Class> otherContainerInstance;


// EOF

I have tested the above with LLVM+Clang 14.0 on Linux and it correctly reports all nine things in the first section and none in the second.

declaratorDecl matches DeclaratorDecl, which is the declaration of anything that uses declarator syntax. Most importantly here, it matches both VarDecl and FieldDecl, whereas varDecl only matches VarDecl. (I'm just assuming you want to match fields too.)

In the AST matcher language, allOf is usually implicit, so I have omitted it from my query, but it also works to have it explicitly.

I chose to use hasName rather than matchesName for greater specificity, but either will work.

For ease of experimentation and reproducibility, my matcher is a clang-query command. Although I have not done so, it should be straightforward to translate it to a C++ LibASTMatchers matcher. I believe the effect of IgnoreUnlessSpelledInSource can be achieved by calling clang::ParentMapContext::setTraversalKind(); see this answer for a little more detail about that.

Upvotes: 4

Related Questions