[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

[Piotr Padlewski via cfe-commits]

Prazek added inline comments.

================
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:25
@@ +24,3 @@
+  Finder->addMatcher(
+      cxxThrowExpr(stmt(hasAncestor(functionDecl(isNoThrow()).bind("func"))))
+          .bind("throw"),
----------------
you can use forFunction instead of hasAncestor(functionDecl(
cxxThrowExpr(stmt(forFunction(isNoThrow() or cxxThrowExpr(stmt(forFunction(functionDecl(isNoThrow())))
you can even try to remove stmt.


Also add test case

  void getFunction() noexcept {
   struct X { 
    static void inner()
    {
        throw 42;
    }
   }; 
  }


================
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:130
@@ +129,3 @@
+  const auto *Throw = Result.Nodes.getNodeAs<Stmt>("throw");
+  diag(Throw->getLocStart(), "throw in function declared no-throw");
+  for (auto Redecl : Function->redecls()) {
----------------
add new line after this line

================
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:131
@@ +130,3 @@
+  diag(Throw->getLocStart(), "throw in function declared no-throw");
+  for (auto Redecl : Function->redecls()) {
+    SourceRange NoExceptRange =
----------------
add some new line

================
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:134
@@ +133,3 @@
+        FindNoExceptRange(*Result.Context, *Result.SourceManager, *Redecl);
+    if (NoExceptRange.isValid()) {
+      diag(NoExceptRange.getBegin(), "In function declared no-throw here:")
----------------
if one one the redecl won't be valid, then you can't change any of them. I guess you should accumulate them in 
llvm::SmallVector<SourseRange, 5> whilie checking if they are valid. If all of them will be valid then perform fix on all of them.

================
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:135
@@ +134,3 @@
+    if (NoExceptRange.isValid()) {
+      diag(NoExceptRange.getBegin(), "In function declared no-throw here:")
+          << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
----------------
I think it would be better to tell about declaration only in one place (where is the definition)

Also, I would use "note:" instead of warning here. (passing  DiagnosticIDs::Level::Note as third argument)
You can also see how to do it here:
http://reviews.llvm.org/D18821

Also I think "no-throw function declared here" would be better


Repository:
  rL LLVM

http://reviews.llvm.org/D19201