This entry is part 4 of 5 in the series Travelling in LLVM land

Compiling C++ code in memory with clang
Address Sanitizer: alternative to valgrind
Book review: LLVM Cookbook
Writing custom checks for clang-tidy
Book review: Getting Started with LLVM Core Libraries

I started taking a heavier interest in clang-tidy a few months ago, as I was looking at static analyzers. I found at the time that it was quite complicated to work on clang internal AST. It is a wonderful tool, but it is also a very complex one. Thankfully, the cfe-dev mailing list is full of nice people.

I also started my journey in the LLVM/clang land with the help of this blog post.

Quick setup

The previous blog post is very great to explain how to setup a build:

git clone http://llvm.org/git/llvm.git
cd llvm/tools/
git clone http://llvm.org/git/clang.git
cd clang/tools/
git clone https://github.com/mbrucher/clang-tools-extra extra
 
cd ../../../
mkdir build && cd build/
cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo ..
make check-clang-tools

A new checker can be created with the following command line:

./add_new_check.py misc catch-by-const-ref

A new folder can easily be created manually, and each checker consists of two sections:

A matcher that will select AST sections
A checker that will add additional checks on top of the matcher, like macro or file

Now let’s try to implement two rules:

The first one will check that we catch exceptions by const ref (a good practice)
The second will allow functions detections (C functions that are now replaced by C++, or functions that should be replaced by safer ones).

A simple matcher for catching by const ref

The best reference for the matchers in clang is (unfortunately) the doxygen for the last_matcher namespace. As you can see, it is quite difficult to navigate, but it’s not as complicated.

Let’s start with what we need to scan:

A variable declaration
Inside a catch statement
That is a reference
But not const

My first trial was to match all catch statement. This is easily done by using cxxCatchStmt. When I did that, the issue that I could check that the variable underneath was declared const or not. So instead, I asked some help from the cfe-dev people.

So let’s start over again. This is what we need:

variable declaration is matched with varDecl (for a type VarDecl, notice the case difference)
inside a catch statement is matched by isExceptionVariable
a reference type is matched by references
and the const aspect is matched by isConstQualified

varDecl is an instance of VariadicDynCastAllOfMatcher that matches VarDecl. It can take several parameters. So the first parameter will be isExceptionVariable. The second will describe that type of access we are looking for hasType(references(qualType(unless(isConstQualified())))). If you unroll this match, we are looking for a reference on a qualifier type that is not (unless) const qualified.

The result is then:

void CatchByConstRefCheck::registerMatchers(MatchFinder *Finder) {

  // This is a C++ only check thus we register the matchers only for C++

```
  if (!getLangOpts().CPlusPlus)
```
```
    return;
```
```
 
```

  Finder->addMatcher(varDecl(isExceptionVariable(),hasType(references(qualType(unless(isConstQualified()))))).bind("catch"), this);

```
}
```

Now that we have a good matcher, the checker is easy to write. We want to warn for all these variables, and we can even easily propose a fix.

void CatchByConstRefCheck::check(const MatchFinder::MatchResult &Result) {

```
 
```

  const VarDecl* varCatch = Result.Nodes.getNodeAs<VarDecl>("catch");

```
 
```

  const char *diagMsgCatchReference = "catch handler catches by non const reference; "

                                        "catching by const-reference may be more efficient";

```
 
```

  // Emit error message if the type is not const (ref)s

  diag(varCatch->getLocStart(), diagMsgCatchReference)

    << FixItHint::CreateInsertion(varCatch->getLocStart(), "const ");

```
}
```

Of course, I’ve written a few examples that are tested by clang testing framework (make check-clang-tools).

Using check options for matching deprecated functions

Now, for a second rule, I wanted to detect some C functions that have a C++ equivalent. For instance, exp() should be replaced by std::exp(), or fabs() by std::abs(). As the list can be different for different projects (and as you may want to replace other functions by others).

When using options, there are two things to do. First getting the options in the constructor, and also use a store call:

DetectCFunctionsCheck::DetectCFunctionsCheck(StringRef Name, ClangTidyContext *Context)

```
    : ClangTidyCheck(Name, Context),
```

      stdNamespaceFunctions(Options.get("stdNamespaceFunctions", "floor,exp")),

      functionsToChange(Options.get("functionsToChange", "fabs>std::abs"))

```
{
```
```
    parseStdFunctions();
```
```
    parseFunctionToChange();
```
```
}
```
```
 
```

void DetectCFunctionsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts)

```
{
```

  Options.store(Opts, "stdNamespaceFunctions", stdNamespaceFunctions);

  Options.store(Opts, "functionsToChange", functionsToChange);

```
}
```

I have two calls here to parse the option strings. They are in charge of splitting them at ‘,’ and then for the replacement functions, we split them at ‘>’. Here, the default options are very simple, and it is easy to change it.

The matchers are very similar:

void DetectCFunctionsCheck::registerMatchers(MatchFinder *Finder) {

    // Should check if there are duplicates.

    for(auto fun: stdNamespaceFunctionsSet)

```
    {
```

      Finder->addMatcher(callExpr(callee(functionDecl(allOf(hasName(fun), unless(cxxMethodDecl()), hasParent(translationUnitDecl()))))).bind(fun), this);

```
    }
```
```
    for(auto fun: functionsToChangeMap)
```
```
    {
```

      Finder->addMatcher(callExpr(callee(functionDecl(allOf(hasName(fun.first), unless(cxxMethodDecl()), hasParent(translationUnitDecl())))).bind(fun.first), this);

```
    }
```
```
}
```

So we select call expression that use a function whose name contains the name required, that is not a call to a method and that the parent of the call is the translation unit a.k.a. the global namespace (one could use namespace here if the function was to be in a namespace). Then the check is very easy as well as the fix-it hint:

```
 
```

void DetectCFunctionsCheck::check(const MatchFinder::MatchResult &Result) {

```
 
```

    for(const auto& fun: stdNamespaceFunctionsSet)

```
    {
```

        const CallExpr* call = Result.Nodes.getNodeAs<CallExpr>(fun);

```
        if(call)
```
```
        {
```

            diag(call->getLocStart(), "this function has a corresponding std version. Consider using it (std::" + fun + ")")

                << FixItHint::CreateInsertion(call->getLocStart(), "std::");

```
        }
```
```
    }
```

    for(const auto& fun: functionsToChangeMap)

```
    {
```

        const CallExpr* call = Result.Nodes.getNodeAs<CallExpr>(fun.first);

```
        if(call)
```
```
        {
```

            auto start = call->getLocStart();

            diag(start, "this function has a better version. Consider using it (" + fun.second + ")")

                << FixItHint::CreateReplacement(SourceRange(start, start.getLocWithOffset(fun.first.size() - 1)), fun.second);

```
        }
```
```
    }
```
```
}
```

It would be easy to add a third category, for instance for C unsafe functions, but I don’t need this for now.

I have additional functional tests as well in the repository.

Conclusion

I like writing rules, as clang-tidy is very powerful. Unfortunately, it is sometimes difficult to figure out what query you want to write. Although clang-query helps on this a lot, it is not very nice to use (there is no history of previous rules, you can’t go back on the same line…). I suppose dumping the AST helps as you can figure out what is the match you really want.

These two rules are available on github.

Series Navigation<< Book review: LLVM CookbookBook review: Getting Started with LLVM Core Libraries >>

Writing custom checks for clang-tidy

Quick setup

A simple matcher for catching by const ref

Using check options for matching deprecated functions

Conclusion

1 thought on “`Writing custom checks for clang-tidy`”

Leave a Reply Cancel reply

Quick setup

A simple matcher for catching by const ref

Using check options for matching deprecated functions

Conclusion

Share this:

1 thought on “Writing custom checks for clang-tidy”

Leave a Reply Cancel reply

1 thought on “`Writing custom checks for clang-tidy`”