hintonda updated this revision to Diff 57686.
hintonda added a comment.

Improved matcher logic and add better range handling to try to deal
with multiple asserts concerning bad ranges when running checker again
real code, e.g., libcxx.

Even so, still seeing some asserts in Lexer::getSourceLocation(), e.g.:

Assertion failed: (Loc >= BufferStart && Loc <= BufferEnd &&
 "Location out of range for this buffer!"), function
 getSourceLocation, file
 /Users/dhinton/projects/clang/llvm/tools/clang/lib/Lex/Lexer.cpp, l
 ine 1073.

in a call to Sources.isBeforeInTranslationUnit(Range.getEnd(),
Tok.getLocation()) in parseDeclTokens().

Looks like my matcher is returning stuff that doesn't parse correctly.
Investigating.


http://reviews.llvm.org/D18575

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseNoexceptCheck.cpp
  clang-tidy/modernize/UseNoexceptCheck.h
  clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tidy/utils/LexerUtils.cpp
  clang-tidy/utils/LexerUtils.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/modernize-use-noexcept.rst
  test/clang-tidy/modernize-use-noexcept-macro.cpp
  test/clang-tidy/modernize-use-noexcept.cpp

Index: test/clang-tidy/modernize-use-noexcept.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s modernize-use-noexcept %t -- \
+// RUN:   -- -std=c++11
+
+class A {};
+class B {};
+
+void foo() throw();
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'foo' uses dynamic exception specification 'throw()' [modernize-use-noexcept]
+// CHECK-FIXES: void foo() noexcept;
+
+void bar() throw(...);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bar' uses dynamic exception specification 'throw(...)' [modernize-use-noexcept]
+// CHECK-FIXES: void bar() noexcept(false);
+
+void foobar() throw(A, B)
+{}
+// CHECK-MESSAGES: :[[@LINE-2]]:6: warning: function 'foobar' uses dynamic exception specification 'throw(A, B)' [modernize-use-noexcept]
+// CHECK-FIXES: void foobar() noexcept(false)
+
+void baz(int = (throw A(), 0)) throw(A, B) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'baz' uses dynamic exception specification 'throw(A, B)' [modernize-use-noexcept]
+// CHECK-FIXES: void baz(int = (throw A(), 0)) noexcept(false) {}
+
+void f(void (*fp)(void) throw()) throw(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' uses dynamic exception specification 'throw()' [modernize-use-noexcept]
+// CHECK-MESSAGES: :[[@LINE-2]]:6: warning: function 'f' uses dynamic exception specification 'throw(char)' [modernize-use-noexcept]
+// CHECK-FIXES: void f(void (*fp)(void) noexcept) noexcept(false);
+
+void g(void (*fp)(void) throw());
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'g' uses dynamic exception specification 'throw()' [modernize-use-noexcept]
+// CHECK-FIXES: void g(void (*fp)(void) noexcept);
+
+void j() throw(int(int) throw(void(void) throw(int)));
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'j' uses dynamic exception specification 'throw(int(int) throw(void(void) throw(int)))' [modernize-use-noexcept]
+// CHECK-FIXES: void j() noexcept(false);
+
+void k() throw(int(int));
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'k' uses dynamic exception specification 'throw(int(int))' [modernize-use-noexcept]
+// CHECK-FIXES: void k() noexcept(false);
+
+// Should not trigger a replacement.
+void titi() noexcept {}
+void toto() noexcept(true) {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s modernize-use-noexcept %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-noexcept.ReplacementString, value: 'NOEXCEPT'}]}" \
+// RUN:   -- -std=c++11
+
+// Example definition of NOEXCEPT -- simplified test to see if noexcept is supported. 
+#if (__has_feature(cxx_noexcept))
+#define NOEXCEPT noexcept
+#else
+#define NOEXCEPT throw()
+#endif
+
+void bar() throw() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bar' uses dynamic exception specification 'throw()' [modernize-use-noexcept]
+// CHECK-FIXES: void bar() NOEXCEPT {}
+
+// Should not trigger a FixItHint, since macros only support noexcept, and this
+// case throws.
+class A {};
+class B {};
+void foobar() throw(A, B);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'foobar' uses dynamic exception specification 'throw(A, B)' [modernize-use-noexcept]
+
+// Should not trigger a replacement.
+void foo() noexcept(true);
Index: docs/clang-tidy/checks/modernize-use-noexcept.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-noexcept.rst
@@ -0,0 +1,55 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+======================
+
+The check converts dynamic exception specifications, e.g., ``throw()``,
+``throw(<exception>[,...])``, or ``throw(...)`` to ``noexcept``, ``noexcept(false)``,
+or a user defined macro.
+
+Example
+-------
+
+.. code-block:: c++
+
+  void foo() throw();
+	void bar() throw(int) {}
+
+transforms to:
+
+.. code-block:: c++
+
+  void foo() noexcept;
+	void bar() noexcept(false) {}
+
+
+User defined macros
+-------------------
+
+By default this check will only replace ``throw()`` with ``noexcept``,
+and ``throw(<exception>[,...])`` or ``throw(...)`` with
+``noexcept(false)``.  Additionally, users can also use
+:option:`ReplacementString` to specify a macro to use instead of
+``noexcept``.  This is useful when maintaining source code that must
+be compiled with older compilers that don't support the ``noexcept``
+keyword.  Users can define the macro to be ``noexcept`` or ``throw()``
+depending on whether or not noexcept is supported.
+
+Please note that since ``throw(int)`` is equivelent to
+``noexcept(false)`` not ``noexcept``, this check will detect, but not
+provide a FixItHint in that case.
+
+Example
+^^^^^^^
+
+.. code-block:: c++
+
+  void foo() throw() {}
+
+transforms to:
+
+.. code-block:: c++
+
+  void foo() NOEXCEPT {}
+
+if the :option:`ReplacementString` option is set to `NOEXCEPT`.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -191,6 +191,11 @@
   Selectively replaces string literals containing escaped characters with raw
   string literals.
 
+- New `modernize-use-noexcept
+  <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-noexcept.html>`_ check
+
+  Replaces dynamic exception specifications with ``noexcept`` or a user defined macro.
+
 - New `performance-faster-string-find
   <http://clang.llvm.org/extra/clang-tidy/checks/performance-faster-string-find.html>`_ check
 
Index: clang-tidy/utils/LexerUtils.h
===================================================================
--- clang-tidy/utils/LexerUtils.h
+++ clang-tidy/utils/LexerUtils.h
@@ -12,6 +12,7 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/SmallVector.h"
 
 namespace clang {
 namespace tidy {
@@ -23,6 +24,10 @@
 Token getPreviousNonCommentToken(const ASTContext &Context,
                                  SourceLocation Location);
 
+SmallVector<Token, 16> parseDeclTokens(const ASTContext &Context,
+                                       const SourceManager &Sources,
+                                       const CharSourceRange &Range);
+
 } // namespace lexer
 } // namespace utils
 } // namespace tidy
Index: clang-tidy/utils/LexerUtils.cpp
===================================================================
--- clang-tidy/utils/LexerUtils.cpp
+++ clang-tidy/utils/LexerUtils.cpp
@@ -35,6 +35,33 @@
   return Token;
 }
 
+SmallVector<Token, 16> parseDeclTokens(const ASTContext &Context,
+                                       const SourceManager &Sources,
+                                       const CharSourceRange &Range) {
+  std::pair<FileID, unsigned> LocInfo =
+      Sources.getDecomposedLoc(Range.getBegin());
+  StringRef File = Sources.getBufferData(LocInfo.first);
+  const char *TokenBegin = File.data() + LocInfo.second;
+  Lexer RawLexer(Sources.getLocForStartOfFile(LocInfo.first),
+                 Context.getLangOpts(), File.begin(), TokenBegin, File.end());
+  SmallVector<Token, 16> Tokens;
+  Token Tok;
+  while (!RawLexer.LexFromRawLexer(Tok)) {
+    if (Tok.is(tok::semi) || Tok.is(tok::l_brace))
+      break;
+    if (Sources.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation()))
+      break;
+    if (Tok.is(tok::raw_identifier)) {
+      IdentifierInfo &Info = Context.Idents.get(StringRef(
+          Sources.getCharacterData(Tok.getLocation()), Tok.getLength()));
+      Tok.setIdentifierInfo(&Info);
+      Tok.setKind(Info.getTokenID());
+    }
+    Tokens.push_back(Tok);
+  }
+  return Tokens;
+}
+
 } // namespace lexer
 } // namespace utils
 } // namespace tidy
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===================================================================
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -10,7 +10,7 @@
 #include "UseOverrideCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Lex/Lexer.h"
+#include "../utils/LexerUtils.h"
 
 using namespace clang::ast_matchers;
 
@@ -24,36 +24,6 @@
     Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this);
 }
 
-// Re-lex the tokens to get precise locations to insert 'override' and remove
-// 'virtual'.
-static SmallVector<Token, 16>
-ParseTokens(CharSourceRange Range, const MatchFinder::MatchResult &Result) {
-  const SourceManager &Sources = *Result.SourceManager;
-  std::pair<FileID, unsigned> LocInfo =
-      Sources.getDecomposedLoc(Range.getBegin());
-  StringRef File = Sources.getBufferData(LocInfo.first);
-  const char *TokenBegin = File.data() + LocInfo.second;
-  Lexer RawLexer(Sources.getLocForStartOfFile(LocInfo.first),
-                 Result.Context->getLangOpts(), File.begin(), TokenBegin,
-                 File.end());
-  SmallVector<Token, 16> Tokens;
-  Token Tok;
-  while (!RawLexer.LexFromRawLexer(Tok)) {
-    if (Tok.is(tok::semi) || Tok.is(tok::l_brace))
-      break;
-    if (Sources.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation()))
-      break;
-    if (Tok.is(tok::raw_identifier)) {
-      IdentifierInfo &Info = Result.Context->Idents.get(StringRef(
-          Sources.getCharacterData(Tok.getLocation()), Tok.getLength()));
-      Tok.setIdentifierInfo(&Info);
-      Tok.setKind(Info.getTokenID());
-    }
-    Tokens.push_back(Tok);
-  }
-  return Tokens;
-}
-
 static StringRef GetText(const Token &Tok, const SourceManager &Sources) {
   return StringRef(Sources.getCharacterData(Tok.getLocation()),
                    Tok.getLength());
@@ -112,7 +82,8 @@
   // FIXME: Instead of re-lexing and looking for specific macros such as
   // 'ABSTRACT', properly store the location of 'virtual' and '= 0' in each
   // FunctionDecl.
-  SmallVector<Token, 16> Tokens = ParseTokens(FileRange, Result);
+  SmallVector<Token, 16> Tokens = utils::lexer::parseDeclTokens(
+      *Result.Context, *Result.SourceManager, FileRange);
 
   // Add 'override' on inline declarations that don't already have it.
   if (!HasFinal && !HasOverride) {
Index: clang-tidy/modernize/UseNoexceptCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/modernize/UseNoexceptCheck.h
@@ -0,0 +1,50 @@
+//===--- UseNoexceptCheck.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_USE_NOEXCEPT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_NOEXCEPT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+using CandidateSet = llvm::StringSet<llvm::MallocAllocator>;
+
+/// \brief Replace dynamic exception specifications, with
+/// `noexcept` (or user-defined macro) or `noexcept(false)`.
+/// \code
+///   void foo() throw();
+///   void bar() throw(A);
+/// \endcode
+/// Is converted to:
+/// \code
+///   void foo() noexcept;
+//    void bar() noexcept(false);
+/// \endcode
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-noexcept.html
+class UseNoexceptCheck : public ClangTidyCheck {
+public:
+  UseNoexceptCheck(StringRef Name, ClangTidyContext *Context);
+  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 DefaultReplacement;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_NOEXCEPT_H
Index: clang-tidy/modernize/UseNoexceptCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/modernize/UseNoexceptCheck.cpp
@@ -0,0 +1,154 @@
+//===--- UseNoexceptCheck.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 "UseNoexceptCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+static StringRef
+makeDynamicExceptionString(const SourceManager &SM,
+                           const CharSourceRange &FileMoveRange) {
+  std::pair<FileID, unsigned> BeginInfo =
+      SM.getDecomposedLoc(FileMoveRange.getBegin());
+  std::pair<FileID, unsigned> EndInfo =
+      SM.getDecomposedLoc(FileMoveRange.getEnd());
+
+  // Add 1 which represents the size of the trailing ')'.
+  auto Len = EndInfo.second - BeginInfo.second + 1;
+  return StringRef(SM.getCharacterData(FileMoveRange.getBegin()), Len);
+}
+
+namespace {
+
+class SimpleDeclParser {
+public:
+  struct Replacement {
+    Replacement(CharSourceRange Range, bool IsNoThrow)
+        : Range(Range), IsNoThrow(IsNoThrow) {}
+    CharSourceRange Range;
+    bool IsNoThrow;
+  };
+
+  SimpleDeclParser(const SourceManager &SM, const ASTContext &Context,
+                   const SourceRange &Range)
+      : Tokens{utils::lexer::parseDeclTokens(Context, SM,
+                                             CharSourceRange(Range, true))} {
+    Current = Tokens.begin();
+    End = Tokens.end();
+    parseExpr();
+  }
+  void parseExpr() {
+    while (Current != End) {
+      if (Current->is(tok::l_paren)) {
+        parseParenExpr();
+      } else if (Current->is(tok::r_paren)) {
+        return;
+      } else if (Current->is(tok::kw_throw)) {
+        parseDynamicExceptionExpr();
+      } else {
+        ++Current;
+      }
+    }
+  }
+  SourceLocation parseParenExpr() {
+    if (Current == End)
+      return SourceLocation();
+    // eat '('
+    if (++Current == End)
+      return SourceLocation();
+    parseExpr();
+    if (!Current->is(tok::r_paren))
+      return SourceLocation();
+    // eat ')'
+    return (Current++)->getLocation();
+    ;
+  }
+  void parseDynamicExceptionExpr() {
+    static int Level = 0;
+    auto BeginLoc = Current->getLocation();
+    // eat 'throw'
+    if (++Current == End)
+      return;
+    if (!Current->is(tok::l_paren))
+      return;
+    ++Level;
+    auto Last = Current;
+    auto EndLoc = parseParenExpr();
+    if (Level == 1) {
+      if (EndLoc.isValid()) {
+        auto Range = CharSourceRange::getTokenRange(BeginLoc, EndLoc);
+        Replacements.push_back(Replacement{Range, (Last + 2 == Current)});
+      }
+    }
+    --Level;
+  }
+
+  const SmallVector<Replacement, 4> &getReplacements() { return Replacements; }
+
+private:
+  SmallVector<Token, 16> Tokens;
+  SmallVector<Token, 16>::const_iterator Current;
+  SmallVector<Token, 16>::const_iterator End;
+
+  SmallVector<Replacement, 4> Replacements;
+};
+
+} // anonymous namespace
+
+UseNoexceptCheck::UseNoexceptCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      DefaultReplacement(Options.get("ReplacementString", "noexcept")) {}
+
+void UseNoexceptCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "ReplacementString", DefaultReplacement);
+}
+
+void UseNoexceptCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+  Finder->addMatcher(functionDecl(anyOf(hasDynamicExceptionSpec(),
+                                        unless(parameterCountIs(0))))
+                         .bind("funcDecl"),
+                     this);
+}
+
+void UseNoexceptCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("funcDecl")) {
+    auto Range = FuncDecl->getSourceRange();
+    if (!Range.isValid())
+      return;
+
+    StringRef SpecificReplacement = DefaultReplacement;
+    SimpleDeclParser Parser(*Result.SourceManager, *Result.Context, Range);
+    for (const auto &I : Parser.getReplacements()) {
+      SpecificReplacement = DefaultReplacement;
+      FixItHint FixIt;
+      if (!I.IsNoThrow)
+        SpecificReplacement = "noexcept(false)";
+      if (I.IsNoThrow || DefaultReplacement == "noexcept")
+        FixIt = FixItHint::CreateReplacement(I.Range, SpecificReplacement);
+
+      diag(FuncDecl->getLocation(), "function %0 uses dynamic exception "
+                                    "specification '%1'")
+          << FuncDecl
+          << makeDynamicExceptionString(*Result.SourceManager, I.Range)
+          << FixIt;
+    }
+  }
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===================================================================
--- clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -21,6 +21,7 @@
 #include "ShrinkToFitCheck.h"
 #include "UseAutoCheck.h"
 #include "UseDefaultCheck.h"
+#include "UseNoexceptCheck.h"
 #include "UseNullptrCheck.h"
 #include "UseOverrideCheck.h"
 
@@ -48,6 +49,7 @@
     CheckFactories.registerCheck<ShrinkToFitCheck>("modernize-shrink-to-fit");
     CheckFactories.registerCheck<UseAutoCheck>("modernize-use-auto");
     CheckFactories.registerCheck<UseDefaultCheck>("modernize-use-default");
+    CheckFactories.registerCheck<UseNoexceptCheck>("modernize-use-noexcept");
     CheckFactories.registerCheck<UseNullptrCheck>("modernize-use-nullptr");
     CheckFactories.registerCheck<UseOverrideCheck>("modernize-use-override");
   }
Index: clang-tidy/modernize/CMakeLists.txt
===================================================================
--- clang-tidy/modernize/CMakeLists.txt
+++ clang-tidy/modernize/CMakeLists.txt
@@ -15,6 +15,7 @@
   ShrinkToFitCheck.cpp
   UseAutoCheck.cpp
   UseDefaultCheck.cpp
+  UseNoexceptCheck.cpp
   UseNullptrCheck.cpp
   UseOverrideCheck.cpp
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to