[clang-tidy] Prototype to check for exception specification

[JonasToth]

Summary

This check is a first prototype to analyze the exception specification of source code.
Its goal is to annotate function correctly and automatically whenever possible and
find places where noexcept-rules are violated.
It is meant to be a basis to check if destructors and deallocation functions are truly
noexcept.

The current state has some Problems and i added notes on code places i dont like, but dont
know how to do better. Its not meant to be merged directly, but more as a discussion
basis.

[JonasToth]

clang-tidy/modernize/NoexceptCorrectnessCheck.cpp

28

I could not figure out how to refactor hasDirectThrow, hasIndirectThrow and hasThrowingExpression properly.
Creating functions that would return these matchers, resulted in compile errors while binding. I could not see the problem in all the template errors. Maybe someone else can tell me how to do this correctly.

60

How can i find out the exception spepcification in a FunctionDecl? I would like to check if noexcept might be added when it is not justified.

[JonasToth]

• [Doc] short descript in header added

[Eugene.Zelenko]

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).

docs/clang-tidy/checks/modernize-noexcept-correctness.rst

8

Please highlight language constructs with ``.

17

Please run Clang-format over all code snippets.

42

Please highlight language constructs with ``.

[mgehre]

clang-tidy/modernize/NoexceptCorrectnessCheck.cpp

49

dynamic_cast can only throw if the new type is a reference type.

60

Try ThrowingDecl ->getType()->getAs<FunctionProtoType>()->getNoexceptSpec()

test/clang-tidy/modernize-noexcept-correctness.cpp

5

I find it helpful to get fixits for adding "noexcept", but not so much for adding noexcept(false) because this is already the default. I would prefer to hide generation of fixits that state the default behind an option.

43

Use FIXME here?

49

Use FIXME here?

[JonasToth]

commented review

clang-tidy/modernize/NoexceptCorrectnessCheck.cpp

60

Does not build :/

note: candidate: clang::FunctionProtoType::NoexceptResult clang::FunctionProtoType::getNoexceptSpec(const clang::ASTContext&) const
NoexceptResult getNoexceptSpec(const ASTContext &Ctx) const;

Where do i get the ASTContext from?

docs/clang-tidy/checks/modernize-noexcept-correctness.rst

42

should i highlight function here and above?
as well exception. I think they are not directly language constructs but more concepts.

test/clang-tidy/modernize-noexcept-correctness.cpp

5

yes. i plan to introduce an option for that later (its in the doc mentioned). but first I try to figure out a solid way to find direct, indirect and nonthrowing functions :)

43

this line shall cause a warning. CHECK-MESSAGE inserted.

[JonasToth]

clang-tidy/modernize/NoexceptCorrectnessCheck.cpp

60

FunctionPrototype has the isNothrow method, but it needs the ASTContext as well.
An alternative solution would be hasNoexceptExceptionSpec(). It would ignore throw() most likely.

[aaron.ballman]

clang-tidy/modernize/NoexceptCorrectnessCheck.cpp

60

Result.Context is the ASTContext you need.

[Eugene.Zelenko]

docs/clang-tidy/checks/modernize-noexcept-correctness.rst

42

No, only noexcept(false).

[mboehme]

docs/clang-tidy/checks/modernize-noexcept-correctness.rst

8

Omit comma before "that".

28

Typo: "Definitely"

test/clang-tidy/modernize-noexcept-correctness.cpp

62

Typos: "too complex", "left alone"

[JonasToth]

this check is somewhat duplicated to a check currently in review, which seemed to be abondened?

https://reviews.llvm.org/D19201

I will read that one first and try to coordinate with the author.

[baloghadamsoftware]

The matcher in D33537 could be reused here as well. Maybe I should make it common, but it is more complex than any of the common matchers.

[JonasToth]

In D31370#768684, @baloghadamsoftware wrote:

The matcher in D33537 could be reused here as well. Maybe I should make it common, but it is more complex than any of the common matchers.

yes, the matcher seems interesting. but i think i will abondon this check and integrate the functionality somewhere else (maybe ur check), since it seems to be too much duplication otherwise.
for now, i dont have a lot of time, so it is stall. hopefully, this will change in the next month or so.

[JonasToth]

exception checking seems to end in the front end for such cases, so nothing to do here anymore. reimplement if there is need later.

clang-tidy/modernize/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyModernizeModule
   AvoidBindCheck.cpp
   DeprecatedHeadersCheck.cpp
   LoopConvertCheck.cpp
   LoopConvertUtils.cpp
   MakeSmartPtrCheck.cpp
   MakeSharedCheck.cpp
   MakeUniqueCheck.cpp
   ModernizeTidyModule.cpp
+  NoexceptCorrectnessCheck.cpp
   PassByValueCheck.cpp
   RawStringLiteralCheck.cpp
   RedundantVoidArgCheck.cpp
   ReplaceAutoPtrCheck.cpp
   ReturnBracedInitListCheck.cpp
   ShrinkToFitCheck.cpp
   UseAutoCheck.cpp
   UseBoolLiteralsCheck.cpp

clang-tidy/modernize/ModernizeTidyModule.cpp

 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "AvoidBindCheck.h"
 #include "DeprecatedHeadersCheck.h"
 #include "LoopConvertCheck.h"
 #include "MakeSharedCheck.h"
 #include "MakeUniqueCheck.h"
+#include "NoexceptCorrectnessCheck.h"
 #include "PassByValueCheck.h"
 #include "RawStringLiteralCheck.h"
 #include "RedundantVoidArgCheck.h"
 #include "ReplaceAutoPtrCheck.h"
 #include "ReturnBracedInitListCheck.h"
 #include "ShrinkToFitCheck.h"
 #include "UseAutoCheck.h"
 #include "UseBoolLiteralsCheck.h"

 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<AvoidBindCheck>("modernize-avoid-bind");
     CheckFactories.registerCheck<DeprecatedHeadersCheck>(
         "modernize-deprecated-headers");
     CheckFactories.registerCheck<LoopConvertCheck>("modernize-loop-convert");
     CheckFactories.registerCheck<MakeSharedCheck>("modernize-make-shared");
     CheckFactories.registerCheck<MakeUniqueCheck>("modernize-make-unique");
+    CheckFactories.registerCheck<NoexceptCorrectnessCheck>(
+        "modernize-noexcept-correctness");
     CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value");
     CheckFactories.registerCheck<RawStringLiteralCheck>(
         "modernize-raw-string-literal");
     CheckFactories.registerCheck<RedundantVoidArgCheck>(
         "modernize-redundant-void-arg");
     CheckFactories.registerCheck<ReplaceAutoPtrCheck>(
         "modernize-replace-auto-ptr");
     CheckFactories.registerCheck<ReturnBracedInitListCheck>(

clang-tidy/modernize/NoexceptCorrectnessCheck.h

+//===--- NoexceptCorrectnessCheck.h - clang-tidy-----------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_NOEXCEPT_CORRECTNESS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_NOEXCEPT_CORRECTNESS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-noexcept-correctness.html
+class NoexceptCorrectnessCheck : public ClangTidyCheck {
+public:
+  NoexceptCorrectnessCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_NOEXCEPT_CORRECTNESS_H

clang-tidy/modernize/NoexceptCorrectnessCheck.cpp

+//===--- NoexceptCorrectnessCheck.cpp - clang-tidy-------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "NoexceptCorrectnessCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+void NoexceptCorrectnessCheck::registerMatchers(MatchFinder *Finder) {
+  // Finds direct throw expression that can occur in functions. This
+  // will ignore if the throw is actually reachable.
+  // Whenever a function directly throws it should be marked noexcept(false)
+  // but must not be noexcept!
+  Finder->addMatcher(
+      functionDecl(
+          hasDescendant(
+              // hasDirectThrow
+              cxxThrowExpr(unless(hasAncestor(cxxTryStmt()))).bind("throw")))
+          .bind("direct_throwing_decl"),
+      this);
+
+  // Check if the function contains anything that could throw and if not, it
+  // can be annotated with noexcept.
+  Finder->addMatcher(
+      functionDecl(
+          unless(anyOf(
+              // hasIndirectThrow
+              anyOf(hasDescendant(
+                        callExpr(callee(functionDecl(unless(isNoThrow()))))),
+                    hasDescendant(cxxMemberCallExpr(
+                        callee(functionDecl(unless(isNoThrow()))))),
+                    hasDescendant(cxxOperatorCallExpr(
+                        callee(functionDecl(unless(isNoThrow()))))),
+                    hasDescendant(cxxConstructExpr(
+                        hasDeclaration(functionDecl(unless(isNoThrow())))))),
+              // hasThrowingExpression
+              anyOf(hasDescendant(cxxDynamicCastExpr()),
+                    hasDescendant(cxxNewExpr())),
+              // hasDirectThrow
+              hasDescendant(cxxThrowExpr(unless(hasAncestor(cxxTryStmt())))))))
+          .bind("nothrow_decl"),
+      this);
+}
+
+void NoexceptCorrectnessCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto N = Result.Nodes;
+  if (const auto *ThrowingDecl =
+          N.getNodeAs<FunctionDecl>("direct_throwing_decl")) {
+    // FIXME how is that done? i did not find a noThrow predicate
+    //if (ThrowingDecl->isNoThrow()) {
+      //diag(ThrowingDecl->getLocation(),
+           //"function is marked noexcept but contains a direct throw");
+    //} else {
+      diag(ThrowingDecl->getLocation(),
+           "function can be marked noexcept(false)");
+    //}
+
+    const auto *ThrowStmt = N.getNodeAs<CXXThrowExpr>("throw");
+    diag(ThrowStmt->getLocStart(), "throw expression here",
+         DiagnosticIDs::Note);
+  } else if (const auto *NoexceptDecl =
+                 N.getNodeAs<FunctionDecl>("nothrow_decl")) {
+    // Check if the function is noexcept or not and adjust its specification.
+    diag(NoexceptDecl->getLocation(), "function can be marked noexcept");
+  }
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang

docs/clang-tidy/checks/list.rst

.. toctree::
    misc-unused-using-decls
    misc-use-after-move
    misc-virtual-near-miss
    modernize-avoid-bind
    modernize-deprecated-headers
    modernize-loop-convert
    modernize-make-shared
    modernize-make-unique
+   modernize-noexcept-correctness
    modernize-pass-by-value
    modernize-raw-string-literal
    modernize-redundant-void-arg
    modernize-replace-auto-ptr
    modernize-return-braced-init-list
    modernize-shrink-to-fit
    modernize-use-auto
    modernize-use-bool-literals

docs/clang-tidy/checks/modernize-noexcept-correctness.rst

+.. title:: clang-tidy - modernize-noexcept-correctness
+
+modernize-noexcept-correctness
+==============================
+
+This check will mark functions as noexcept if that is possible.
+`noexcept` is a new keyword in C++ 11 to signal, that no exception will escape from 
+the function annotated with it. Some examples to show how this new feature is used are below.
+
+
+.. code-block:: c++
+    
+    // Executing only `noexcept` operations.
+    int simple_calculation() { // annotate with `noexcept`
+        int i = 0;
+        i+= 42;
+        return i;
+    }
+    
+    // Calling only `noexcept` functions.
+    int complex_calculation() { // annotate with `noexcept` as well
+        int j = 10;
+        j+= simple_calculation(); // this call is `noexcept`
+        return j;
+    }
+
+    // Definitly throw an exception on one path. The check will not analyze if this
+    // path is actually executed (in this function it wouldn't!).
+    int throwing_calculation() { // possibly annotate with `noexcept(false)`
+        int i = 0;
+        i+= 42;
+        if(i == 0) {
+            throw std::runtime_error{"Calculation is errorneous."};
+        }
+        return i;
+    }
+
+
+.. options::
+
+    - AnnotateThrow -> boolean if a function that can throw for sure will be annotated with `noexcept(false)`

test/clang-tidy/modernize-noexcept-correctness.cpp

+// RUN: %check_clang_tidy %s modernize-noexcept-correctness %t
+
+float throwing(float x) {
+  // CHECK-MESSAGES: :[[@LINE-1]]: 7: warning: function can be marked noexcept(false)
+  // CHECK-MESSAGES: :[[@LINE+2]]: 5: note: throw expression here
+  if (x == 0.f)
+    throw "Division by Zero!";
+  else
+    return 42.f / x;
+}
+
+int possibly_noexcept() {
+  // CHECK-MESSAGES: :[[@LINE-1]]: 5: warning: function can be marked noexcept
+  int i = 0;
+  i += 42;
+  return i;
+}
+
+int actually_noexcept() noexcept { // Ok
+  int i = 0;
+  i += 42;
+  return i;
+}
+
+int wrongly_noexcept() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]: 1: warning: function is marked noexcept but contains a direct throw
+  // CHECK-MESSAGES: :[[@LINE+4]]: 5: note: throw expression here
+  int i = 0;
+  i += 42;
+  if (i == 42)
+    throw int{15};
+  return i;
+}
+
+int complex_possibly_noexcept() {
+  // CHECK-MESSAGES: :[[@LINE-1]]: 1: warning: function can be marked noexcept
+  int k = 0;
+  k = actually_noexcept() + 15;
+  return k;
+}
+
+// Could throw indirectly.
+float complex_wrongly_noexcept(float x, float y) noexcept {
+  x += throwing(y);
+  return x;
+}
+
+// Could be marked noexcept, but wont right now.
+float complex_non_throwing(float x, float y) {
+  try {
+    if (y == 0.f)
+      throw float();
+    else
+      return x / y;
+  } catch (float) {
+    return x - y;
+  }
+}
+
+// This one is hard, since one must figure out if all possible exceptions are handled.
+// I think that is to complex (for now) and should be let alone. So the programmer
+// must ensure its correctly either marked noexcept or not.
+float case_dependent_throw() { // Ok
+  try {
+    return throwing(10.f);
+  } catch (int) {
+    // handle the exception
+    return 42.f;
+  }
+}