[PATCH] D31370: [clang-tidy] Prototype to check for exception specification

[Jonas Toth via Phabricator via cfe-commits]

JonasToth planned changes to this revision.
JonasToth marked 2 inline comments as done.
JonasToth added a comment.

commented review



================
Comment at: clang-tidy/modernize/NoexceptCorrectnessCheck.cpp:60
+          N.getNodeAs<FunctionDecl>("direct_throwing_decl")) {
+    // FIXME how is that done? i did not find a noThrow predicate
+    //if (ThrowingDecl->isNoThrow()) {
----------------
mgehre wrote:
> JonasToth wrote:
> > 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.
> Try `ThrowingDecl ->getType()->getAs<FunctionProtoType>()->getNoexceptSpec()`
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?


================
Comment at: docs/clang-tidy/checks/modernize-noexcept-correctness.rst:41
+
+    - AnnotateThrow -> boolean if a function that can throw for sure will be annotated with `noexcept(false)`
----------------
Eugene.Zelenko wrote:
> Please highlight language constructs with ``.
should i highlight `function` here and above?
as well `exception`. I think they are not directly language constructs but more concepts.


================
Comment at: test/clang-tidy/modernize-noexcept-correctness.cpp:4
+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
----------------
mgehre wrote:
> 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.
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 :)


================
Comment at: test/clang-tidy/modernize-noexcept-correctness.cpp:42
+
+// Could throw indirectly.
+float complex_wrongly_noexcept(float x, float y) noexcept {
----------------
mgehre wrote:
> Use `FIXME` here?
this line shall cause a warning. CHECK-MESSAGE inserted.


Repository:
  rL LLVM

https://reviews.llvm.org/D31370