[clang-tidy] Fix clang-tidy crashes when using -fdelayed-template-parsing.

[etienneb]

Summary

Fix crashes caused by deferencing null pointer when declarations parsing may be delayed.

The body of the declarations may be null.

The crashes were observed on a Windows build.

command-line switches: -fms-compatibility-version=19 -fms-compatibility

To reproduce them, run clang-tidy over the following basic file:

#include <string>
int main() {}

Templated functions that aren't used may contains a "null" body even if the decl->hasBody() is returning true.

template <typename T> void f() {}


FunctionTemplateDecl 0xd06340 <C:\src\llvm\examples\test.cc:1:1, col:34> col:28 f
|-TemplateTypeParmDecl 0xd061e8 <col:11, col:20> col:20 typename T
`-FunctionDecl 0xd062d0 <col:23, col:34> col:28 f 'void (void)'
  `-<<<NULL>>>

[aaron.ballman]

cppcoreguidelines/ProTypeMemberInitCheck.cpp

183

Please do not use auto here since the type isn't spelled out in the initializer. (The line directly above it also should not be using auto either.)

modernize/RedundantVoidArgCheck.cpp

106–107

Don't use auto here either.

107

You could get rid of the if and else and just use it directly in removeVoidArgumentTokens().

removeVoidArgumentTokens(Result, SourceRange(Start, Body ? Body->getLocStart().getLocWithOffset(-1) : Function-getLocEnd()), "function definition");

[etienneb]

fixed. thanks.

[LegalizeAdulthood]

LGTM

[aaron.ballman]

LGTM, thank you!

[aaron.ballman]

Oops, I take that LGTM back. ;-) Almost LGTM -- this is missing test cases that exercise the behavior.

[etienneb]

tests added.

[rnk]

lgtm, thanks!

[aaron.ballman]

LGTM now (this time for sure), thank you!

[aaron.ballman]

Commit in r264049, thank you!

cppcoreguidelines/ProTypeMemberInitCheck.cpp

Finder->addMatcher(cxxConstructorDecl(isDefinition(), isUserProvided(),
                          .bind("ctor"),
                      this);
 }
 
 void ProTypeMemberInitCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
   const auto &MemberFields = Ctor->getParent()->fields();
 
+  // Skip delayed template instantiation declarations.
+  const Stmt *Body = Ctor->getBody();
+  if (!Body)
+    return;
+
   SmallPtrSet<const FieldDecl *, 16> FieldsToInit;
   fieldsRequiringInit(MemberFields, FieldsToInit);
   if (FieldsToInit.empty())
     return;
 
   for (CXXCtorInitializer *Init : Ctor->inits()) {
     // Return early if this constructor simply delegates to another constructor
     // in the same class.
     if (Init->isDelegatingInitializer())
       return;
     if (!Init->isMemberInitializer())
       continue;
     FieldsToInit.erase(Init->getMember());
   }
-  removeFieldsInitializedInBody(*Ctor->getBody(), *Result.Context,
-                                FieldsToInit);
+  removeFieldsInitializedInBody(*Body, *Result.Context, FieldsToInit);
+
   if (FieldsToInit.empty())
     return;
 
   DiagnosticBuilder Diag =
       diag(Ctor->getLocStart(),
            "constructor does not initialize these built-in/pointer fields: %0")
       << toCommaSeparatedString(MemberFields, FieldsToInit);
   // Do not propose fixes in macros since we cannot place them correctly.

modernize/RedundantVoidArgCheck.cpp

void RedundantVoidArgCheck::check(const MatchFinder::MatchResult &Result) {
   } else if (const auto *Lambda = Nodes.getNodeAs<LambdaExpr>(LambdaId)) {
     processLambdaExpr(Result, Lambda);
   }
 }
 
 void RedundantVoidArgCheck::processFunctionDecl(
     const MatchFinder::MatchResult &Result, const FunctionDecl *Function) {
   SourceLocation Start = Function->getLocStart();
   if (Function->isThisDeclarationADefinition()) {
-    SourceLocation End;
-    if (Function->hasBody())
-      End = Function->getBody()->getLocStart().getLocWithOffset(-1);
-    else
-      End = Function->getLocEnd();
-    removeVoidArgumentTokens(Result, SourceRange(Start, End),
-                             "function definition");
+    const Stmt *Body = Function->getBody();
+    SourceRange range(Start, Body ? Body->getLocStart().getLocWithOffset(-1) :
+                                    Function->getLocEnd());
+    removeVoidArgumentTokens(Result, range, "function definition");
   } else {
     removeVoidArgumentTokens(Result, Function->getSourceRange(),
                              "function declaration");
   }
 }
 
 void RedundantVoidArgCheck::removeVoidArgumentTokens(
     const ast_matchers::MatchFinder::MatchResult &Result, SourceRange Range,