[PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

[don hinton via cfe-commits]

hintonda added inline comments.

================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:97
@@ +96,3 @@
+void UseNoexceptCheck::check(const MatchFinder::MatchResult &Result) {
+  auto FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("functionDecl");
+  if (!FuncDecl)
----------------
etienneb wrote:
> nit:  const auto FuncDecl* 
I originally had const *FuncDecl, but was told to use auto instead.  I'll go ahead and make the change you suggest, but prefer to be explicit unless the type if hard to discern.

================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:120
@@ +119,3 @@
+      << FixItHint::CreateReplacement(
+             CharSourceRange::getTokenRange(FileMoveRange.getBegin(),
+                                            FileMoveRange.getEnd()),
----------------
etienneb wrote:
> FileMoveRange is a CharSourceRange, so why to you need to do getTokenRange?
> 
> ```
>  CharSourceRange FileMoveRange;
> ```
I'm probably missing something, but if I return FileMoveRange, i'm one token short, i.e., it doesn't cover the entire range I'm interested in.

However, if I extract begin and end, it works.  I'll look into the difference, but if you have a better solution, please let me know.

================
Comment at: clang-tidy/utils/LexerUtils.cpp:38
@@ -37,1 +37,3 @@
 
+SmallVector<Token, 16> ParseTokens(const ASTContext &Context,
+                                   const SourceManager &Sources,
----------------
etienneb wrote:
> I like having lexer functions lifted out.
> But 'ParseTokens' doesn't reflect that it stop when reaching a semicolon or a brace.
> 
> ```
> if (Tok.is(tok::semi) || Tok.is(tok::l_brace))
> ```
Not sure what you mean.  Do you want me to change the name or add a comment to make that explicit?


http://reviews.llvm.org/D18575