[clang-tidy] Fix potential assert in use-noexcept check

[njames93]

Summary

Fix a potential assert in use-noexcept check if there is an issue getting the TypeSourceInfo as well as a small clean up.

[njames93]

Figured out the actual cause of this bug, getExceptionSpecRange() returns a null range if the function has an unknown return type

undefined_type throws() throw();

This is the tidy output (where assertions are disabled)

warning: dynamic exception specification '' is deprecated; consider using 'noexcept' instead [modernize-use-noexcept]
/home/ce/example.cpp:1:1: error: unknown type name 'undefined_type' [clang-diagnostic-error]
undefined_type throws() throw();
^
1 warning and 1 error generated.
Error while processing /home/ce/example.cpp.
Found compiler error(s).

[aaron.ballman]

LGTM, but please add a test case for the changes.

[njames93]

• Isolated exact cause of the assert.

[njames93]

In D80371#2052069, @aaron.ballman wrote:

LGTM, but please add a test case for the changes.

As this fix is preventing a crash in error causing code I can't include a specific test case as the clang-tidy tests will fail if there is any error emitted from clang.

[aaron.ballman]

In D80371#2052454, @njames93 wrote:

In D80371#2052069, @aaron.ballman wrote:

LGTM, but please add a test case for the changes.

As this fix is preventing a crash in error causing code I can't include a specific test case as the clang-tidy tests will fail if there is any error emitted from clang.

We support that case as well. See test/clang-tidy/checkers/misc-unused-using-decls-errors.cpp as an example.

[njames93]

• Added test case

clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.h

bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
     return LangOpts.CPlusPlus11;
   }
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
   const std::string NoexceptMacro;
-  bool UseNoexceptFalse;
+  const bool UseNoexceptFalse;
 };
 
 } // namespace modernize
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_NOEXCEPT_H

clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.cpp

 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace modernize {
 
+namespace {
+AST_MATCHER(NamedDecl, isValid) { return !Node.isInvalidDecl(); }
+} // namespace
+
 UseNoexceptCheck::UseNoexceptCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       NoexceptMacro(Options.get("ReplacementString", "")),
       UseNoexceptFalse(Options.get("UseNoexceptFalse", true)) {}
 
 void UseNoexceptCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "ReplacementString", NoexceptMacro);
   Options.store(Opts, "UseNoexceptFalse", UseNoexceptFalse);
 }
 
 void UseNoexceptCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       functionDecl(
-          cxxMethodDecl(
-              hasTypeLoc(loc(functionProtoType(hasDynamicExceptionSpec()))),
-              anyOf(hasOverloadedOperatorName("delete[]"),
-                    hasOverloadedOperatorName("delete"), cxxDestructorDecl()))
-              .bind("del-dtor"))
-          .bind("funcDecl"),
-      this);
-
-  Finder->addMatcher(
-      functionDecl(
-          hasTypeLoc(loc(functionProtoType(hasDynamicExceptionSpec()))),
-          unless(anyOf(hasOverloadedOperatorName("delete[]"),
-                       hasOverloadedOperatorName("delete"),
-                       cxxDestructorDecl())))
+          isValid(),
+          optionally(cxxMethodDecl(anyOf(hasAnyOverloadedOperatorName(
+                                             "delete[]", "delete"),
+                                         cxxDestructorDecl()))
+                         .bind("del-dtor")))
           .bind("funcDecl"),
       this);
 
   Finder->addMatcher(
       parmVarDecl(anyOf(hasType(pointerType(pointee(parenType(innerType(
                             functionProtoType(hasDynamicExceptionSpec())))))),
                         hasType(memberPointerType(pointee(parenType(innerType(
                             functionProtoType(hasDynamicExceptionSpec()))))))))
if (const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("funcDecl")) {
 
     if (const auto *TSI = ParmDecl->getTypeSourceInfo())
       Range = TSI->getTypeLoc()
                   .getNextTypeLoc()
                   .IgnoreParens()
                   .castAs<FunctionProtoTypeLoc>()
                   .getExceptionSpecRange();
   }
+
+  assert(Range.isValid() && "Exception Source Range is invalid.");
+
   CharSourceRange CRange = Lexer::makeFileCharRange(
       CharSourceRange::getTokenRange(Range), *Result.SourceManager,
       Result.Context->getLangOpts());
 
   assert(FnTy && "FunctionProtoType is null.");
   bool IsNoThrow = FnTy->isNothrow();
   StringRef ReplacementStr =
       IsNoThrow

clang-tools-extra/test/clang-tidy/checkers/modernize-use-noexcept-error.cpp

+// RUN: %check_clang_tidy -expect-clang-tidy-error %s modernize-use-noexcept %t
+
+// We're not interested in the check issuing a warning here, just making sure
+// clang-tidy doesn't assert.
+undefined_type doesThrow() throw();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: error: unknown type name 'undefined_type' [clang-diagnostic-error]