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

[hintonda]

Summary

This checker allows users to replace deprecated throw()
specifications with noexcept.

It also allows users to specify a user defined macro to be used
instead of noexect, which is useful when supporting older compilers.
However, the macro will not be used for noexcect(false) since that
doesn't make sense.

[aaron.ballman]

clang-tidy/modernize/UseNoexceptCheck.cpp

25

Why 5?

34

This should not be at file scope; if it really clarifies the code, it should be at function scope where needed.

66

Instead of indenting a considerable amount of code, I think an early return may be a bit more clear.

clang-tidy/modernize/UseNoexceptCheck.h

22

I think you mean "Replace dynamic exception specifications" instead of "noexcept specifications".

[Eugene.Zelenko]

It's much better to add information in release notes in same patch as check itself. See also D18509 for related add_new_check.py improvement.

[hintonda]

Sounds like a good idea. I'll add the additional transformations you mentioned and remove s/noexcept(true)/noexcept/.

clang-tidy/modernize/UseNoexceptCheck.cpp

25

No particular reason -- copied basic implementation from utils::parseHeaderFileExtensions() which did something similar.

34

Will remove/refactor -- was just following the examples I found in other checkers.

84

Ah, that helps a lot. I'll use getExceptionSpecType(), but will still need to get the location of the end of the token sequence for replacement purposes, e.g., throw() is 3 tokens.

[hintonda]

Address comments and refactor.

[hintonda]

Addressed comments and believe it's ready for review.

[alexfh]

clang-tidy/modernize/UseNoexceptCheck.cpp

17

Yes, it might make sense to move it to ASTMatchers.h.

32

Please rename the matcher to hasDynamicExceptionSpec().

36

if (const auto *FnTy = Node.getType()->getAs<FunctionProtoType>())
  return FnTy->hasDynamicExceptionSpec();
return false;

62

I suspect that repeated calls to the GetBeginningOfToken method might be rather expensive. An alternative (and possibly more efficient) approach would be to re-lex the range from the start of the function declaration to the current location (in the forward direction) and then just operate on the array of tokens. See UseOverrideCheck.cpp, for example. If you decide to go that way, we could move the ParseTokens function to clang-tidy/utils/LexerUtils.{h,cpp}.

85

We usually add using namespace ast_matchers; to make matchers more readable.

111

I don't like this use of Tok as an alias of Lexer::Tok. It makes the code using it harder to understand.

124

Please remove the commented-out code or uncomment it, if it's needed.

157

The format of the message is not consistent with other messages. I'd suggest something along the lines of: "function '%0' uses dynamic exception specification '%1'; use '%2' instead".

clang-tidy/modernize/UseNoexceptCheck.h

36

As soon as a constructor starts doing something but just calling the base constructor (and sometimes before that), we usually move it to a .cpp file.

Please also move the storeOptions definition to the .cpp file.

[aaron.ballman]

clang-tidy/modernize/UseNoexceptCheck.cpp

17

Yes, please (along with tests and documentation, etc).

95

Can use auto here to not repeat the type name from the initializer.

149

I think all of this custom logic can be replaced by looking at the FunctionType. If it has a dynamic exception specification (hasDynamicExceptionSpec()) then whether it is throwing or not throwing (isNothrow()) determines whether you replace with noexcept(false) or noexcept. But then you don't have to go back to the source to see what is written.

[hintonda]

I moved over the weekend, but will try to take a look at this tonight and address all your comments. Thanks again...

[hintonda]

Addressed most comments.

[hintonda]

clang-tidy/modernize/UseNoexceptCheck.cpp

62

Still need to fix the call in UseOverrideCheck.cpp, but will wait till this has been reviewed.

132

I still need to parse it to find the range, so I'm not sure I'm not convinced the code will be any shorter/faster, but if there is a faster way, please let me know.

[alexfh]

clang-tidy/modernize/UseNoexceptCheck.cpp

18

Please send a separate patch adding this matcher to ASTMatchers.h (with tests and docs).

74

http://llvm.org/docs/CodingStandards.html#do-not-use-braced-initializer-lists-to-call-a-constructor

Use the least powerful tool for the job: std::string Replacement = ReplacementStr;, this syntax makes it obvious that ReplacementStr is a std::string (or is implicitly converted to std::string).

Same elsewhere.

80

1. Variable names should be CamelCase.
2. It's more common to use I for array indices or iterators in LLVM.

109

Braces should be used consistently on both sides of else if.

115

CamelCase

117

Looks like Lexer::makeFileCharRange will do this job better.

122

Just FuncDecl should be enough.

clang-tidy/modernize/UseNoexceptCheck.h

22

Please enclose inline code snippets in backquotes.

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

32

There's an alternative approach: the appropriate compatibility macro can be detected automatically using the Preprocessor::getLastMacroWithSpelling method. This approach will only work, if the header defining the macro is already included. However, if it's not included, the check must also add the corresponding #include statement to avoid breaking the code.

Also, if you detect the macro name automatically, you can provide a default macro by the means of the command line options (-extra-arg=-DNOEXCEPT=noexcept(false) or the same via the configuration file).

[hintonda]

Addressed additional comments.

[hintonda]

• Fix typo in docs.

[alexfh]

clang-tidy/modernize/UseNoexceptCheck.cpp

40

s/FunctionDecl/auto/

41

s/clang:://

50–51

These two seem to represent a SourceRange.

52

s/std::string/StringRef/

55–91

I'd pull this to a separate function to make the check() method easier to read.

94

Lexer::makeFileCharRange is a convenient way to ensure a range is a contiguous range in the same file.

[hintonda]

Created helper function and moved most logic there. Also address a
few comments.

[alexfh]

clang-tidy/modernize/UseNoexceptCheck.cpp

37

The name "helper" doesn't say much. I'm sure we can come up with a much more useful name. The interface could also be more convenient. It looks like this function could return a FixItHint (which could possibly be isNull(), if no replacement is possible/needed). It could also use an Optional<FixItHint> or some other way to inform the caller that there's no action required.

[hintonda]

clang-tidy/modernize/UseNoexceptCheck.cpp

37

Happy to rename it, but not sure making it more convenient serves much of a purpose. For this checker, we either find a dynamic exception specification and produce a FixItHint, or we don't.

Did you have some other behavior you wanted me to add?

[hintonda]

• Renamed private helper method

[Eugene.Zelenko]

docs/ReleaseNotes.rst

152

Please highlight noexcept with ``.

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

7

Please highlight throw() and other keywords with ``.

[hintonda]

• Added ``'s around keywords in docs.

[Eugene.Zelenko]

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

33

Sorry, missed last time. Please use ` for check option and its value(s). Same for last line in file.

[hintonda]

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

33

I'm not sure I understand what you want me to change. Did you want me to append that phrase, or replace something?

[Eugene.Zelenko]

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

33

Replace :option:`ReplacementString with :option:ReplacementString`. Same in last line with option and NOEXCEPT.

'' is for language constructs, ' is for command line options and their parameters.

[hintonda]

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

33

Ah, got it. Sorry for being so dense... Will checkin shortly...

[hintonda]

• Fix quote problem in docs.

[Eugene.Zelenko]

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

51

Actually :option: still need to prepend ReplacementString.

[hintonda]

• Add :option: prefix for clarity.

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

51

I'll get it eventually... ;-)

[etienneb]

drive by

clang-tidy/modernize/UseNoexceptCheck.cpp

41

nit: const

98

nit: const auto FuncDecl*

115

I'm not sure, but I think you can replace

FuncDecl->getNameInfo().getAsString() by FuncDecl.

IIRC, there is a automatic conversion for the name.

121

FileMoveRange is a CharSourceRange, so why to you need to do getTokenRange?

CharSourceRange FileMoveRange;

clang-tidy/utils/LexerUtils.cpp

37

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))

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

5

nits: space after A and B
class A {};

[hintonda]

• Addressed a few comments for clarity/readability.

[hintonda]

clang-tidy/modernize/UseNoexceptCheck.cpp

98

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.

121

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.

clang-tidy/utils/LexerUtils.cpp

37

Not sure what you mean. Do you want me to change the name or add a comment to make that explicit?

[etienneb]

clang-tidy/modernize/UseNoexceptCheck.cpp

99

I'm not saying to use 'const *FuncDecl', but 'const auto*'

clang-tidy/utils/LexerUtils.cpp

37

If you call ParseTokens, it will parse tokens until it reach a semicolon/lbrace? exact?

[hintonda]

clang-tidy/modernize/UseNoexceptCheck.cpp

100

Sorry, I mis-typed. I meant to say I had, and prefer, "const FunctionDecl*" to "const auto *" in cases like this, but will do whatever you want.

clang-tidy/utils/LexerUtils.cpp

37

Yes, but again, I'm not sure what you are asking me to do/change.

Are you asking me to change something?

[hintonda]

Regenerated diff to make sure I pick up all changed files.

[hintonda]

clang-tidy/utils/LexerUtils.cpp

37

I regenerated the diff -- the last few arc diff commands left out a few files.

Anyway, as you can see, I lifted the ParseTokens() functions out of UseOverrideCheck.cpp and moved it here.

[aaron.ballman]

clang-tidy/modernize/UseNoexceptCheck.h

45–46

What is the difference between these two fields? The names are kind of confusing to me given their similarity, so perhaps renaming one to be more descriptive would be useful.

[hintonda]

clang-tidy/modernize/UseNoexceptCheck.h

45–46

The first is the option passed in, i.e., the replacement string we will use (either noexcept or a user macro like libcxx's _NOEXCEPT) for throw() and the second one is the string we'll actually use for a particular replacement, which can be what the user supplied, or noexcept(false) when the function can throw.

This was originally done with an auto in check(), but Alex wanted me to add a helper function to make check() easier to understand. So, I had to put it somewhere. I actually prefer the simpler original version.

I'll see if I can come up with better names, but suggestions are always welcome.

[alexfh]

clang-tidy/modernize/UseNoexceptCheck.cpp

38

Happy to rename it, but not sure making it more convenient serves much of a purpose.

Well, the initial problem was that the check method was too long and complicated. I suggested to make it easier to read by pulling a part of it in a separate method. I pointed to a part that seemed like something worth extracting to a method, and you mechanically created a method with unclear responsibilities, an awkward interface (two output parameters, a bool return value and a class data member) and a name that doesn't help understanding what the method does. That doesn't make the code any better. Let's try once again. We need to pull out an abstraction (at least one, but maybe you see more) that will make sense on its own and will have a reasonable interface. It could be SourceRange findDynamicExceptionSpecification(ArrayRef<Token> Tokens), for example.

122

I think, the problem is in the way you assign MoveRange in the checkHelper method. It is created as a character range and then you just assign its begin and end location. Instead, try MoveRange = CharSourceRange::getTokenRange(I->getLocation(), Loc).

clang-tidy/modernize/UseNoexceptCheck.h

45–46

The StringRef Replacement; should not be a member, since it's just one of the results of the checkHelper method.

[hintonda]

I agree -- things got messy.

I'm in the process of completely refactoring the whole thing -- including the ast matchers that I'll checkin later today. Thanks for your patience.

[hintonda]

• Complete refactor

[hintonda]

• Make helper functions static.

[hintonda]

• Added asserts for indexes into Tokens array.

[hintonda]

• A bit more refactoring and cleanup.

[hintonda]

I think this is now ready for review.

Thanks again for your patience...

[etienneb]

clang-tidy/modernize/UseNoexceptCheck.cpp

23

coding style:
MakeDynamicExceptionString -> makeDynamicExceptionString

35

MakeMoveRange -> makeMoveRange

(and all other occurrences below)

[hintonda]

• Fix some function names.

[hintonda]

• Revert back to previous version of the matcher and use Aaron's syntax with allOf and unless to achieve the same results -- much simpler.

[aaron.ballman]

clang-tidy/modernize/UseNoexceptCheck.cpp

22

Instead of a bunch of static functions, would an unnamed namespace make more sense?

35

Since this function is only called one time and is a one-liner, perhaps it makes more sense to inline the body at the call site?

47

Pathologically terrible counter-case: void func() throw(decltype(throw 12) *)

50

Can we use >= 0 instead of != -1? It makes it more immediately obvious that the array index will not underflow.

101

Please don't use auto here; I have no idea what type FileMoveRange will have from inspecting the code.

109

No need to quote %0, the diagnostics engine will already do it when passed a NamedDecl, so this will improperly double-quote the diagnostic.

clang-tidy/modernize/UseNoexceptCheck.h

23

I think this comment means noexcept(false) instead? Or is there a reason to replace with noexcept(true) instead of just noexcept?

test/clang-tidy/modernize-use-noexcept-macro.cpp

12

I may have missed some context in the discussion, but why shouldn't this trigger a FixItHint?

[hintonda]

clang-tidy/modernize/UseNoexceptCheck.cpp

22

Just following the pattern established in other checkers so as to minimize the number of changes, but will change to namespace.

35

Was a bit more involved, but last round of changes simplified it. Will simplify even more.

47

Good point, looks like I need a full fledged parser to catch 100% or the cases -- or we could ignore these corner cases.

clang-tidy/modernize/UseNoexceptCheck.h

23

Typo, should just be noexcept.

[aaron.ballman]

clang-tidy/modernize/UseNoexceptCheck.cpp

22

I don't have strong opinions; I think the usual rule of thumb boils down to whether the code fits on a page or not (if it does, use an unnamed namespace, if not, use static functions).

47

At the very least, we should have test cases showing what the behavior is with a big FIXME around this code, should you decide to keep it. I'm not keen on the idea of this being part of a fixit that may destroy well-defined user code. Same for the assumptions about the location of right parens. That code looks equally broken even without multiple throw tokens in the stream. Consider:
void func() throw(int(int));

[hintonda]

clang-tidy/modernize/UseNoexceptCheck.cpp

47

Had thought about adding paren parsing, but wasn't sure it was needed -- thanks for pointing out that it is. Of course, I'll have to parse from the beginning to do this correctly, but that's not a big deal.

50

Sure.

test/clang-tidy/modernize-use-noexcept-macro.cpp

12

Because the user provided a macro. The only reason you would do that is if you want the macro to expand to something different depending on whether or not 'noexcept' is supported.

Perhaps this can be done more elegantly, but the use case for this entire checker was libcxx. It defines _NOEXCEPT as either noexcept or throw() depending on whether or not noexcept is supported. I don't see a good way of doing that, other than removing it completely, so I just reported it without supplying a FixItHint.

[aaron.ballman]

test/clang-tidy/modernize-use-noexcept-macro.cpp

12

Ah, thank you for the context! That information should probably go in the user facing documentation somewhere.

[hintonda]

• Address additional comments.

[aaron.ballman]

clang-tidy/modernize/UseNoexceptCheck.cpp

40

@alexfh, what are your feelings on this FIXME? I am a bit concerned because these examples will cause the replacement range to be incorrect, which will turn working code into ill-formed code. The alternative, as I see it, is to instead properly track the exception specification source range information as part of the FunctionDecl (akin to FunctionDecl::getReturnTypeSourceRange()).

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

41

This seems to run contrary to one of the examples below where a fixit is provided for `throw(A, B)`. If I understood properly, this statement is only true when using a ReplacementString. Perhaps it should instead say, "this check will detect, but not
provide a FixItHint for this case when a :option:ReplacementString is provided."

[hintonda]

clang-tidy/modernize/UseNoexceptCheck.cpp

40

Btw, I'm working on a fix I believe will handle all cases -- plan to checkin later today. However, it won't be that efficient unless I can find a way to match params that contain dynamic exception specifications. If they are only legal for function pointers -- which I think is the case -- that would make it easy and efficient, i.e., I wouldn't have to match all FunctionDecl's with one or more parameter and test them.

Is it possible to match a parameter that is a function pointer?

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

41

This is only applicable when the user provided a replacement string other than noexcept. I'll try to make it clearer, however, there is a FIXME in the code concerning this. Specifically, should we allow replacement options for both noexcept and noexcept(false).

[alexfh]

clang-tidy/modernize/UseNoexceptCheck.cpp

23

It's much more common for LLVM code to use static functions:

http://llvm.org/docs/CodingStandards.html#anonymous-namespaces
"make anonymous namespaces as small as possible, and only use them for class declarations"

40

The alternative, as I see it, is to instead properly track the exception specification source range information as part of the FunctionDecl (akin to FunctionDecl::getReturnTypeSourceRange()).

It's all trade-offs: adding this information to the AST allows certain tasks in tooling to be done easier. On the other hand, this leads to bloating of the AST, which can already be huge. In this specific case, always keeping the source range of the exception specifier might be wasteful, since exception specifiers are rather rare (in my world, at least). But there might be a way to optionally store this information, e.g. in the ASTContext. In any case, makes sense to ask Richard Smith.

[aaron.ballman]

clang-tidy/modernize/UseNoexceptCheck.cpp

40

I am more curious as to your comfort level about committing a fixit that we know will break working code. ;-)

As for the suggested approach, I would obviously cover that with Richard, but I think we're going to need exception specifications more now that they're going to be part of the type system for C++. We have FunctionProtoTypeLoc for tracking this information.

[alexfh]

clang-tidy/modernize/UseNoexceptCheck.cpp

40

The important question here is how frequently is this likely to happen. I guess, it depends on the codebase, but maybe you have a rough guess?

[aaron.ballman]

clang-tidy/modernize/UseNoexceptCheck.cpp

40

Multiple uses of throw within a dynamic exception specification? Rare. Paren use that doesn't match what we expect? Infrequent, but likely to generate a bug report at some point depending on how often people use clang-tidy to check extensive code bases that actually *use* dynamic exception specifications. So my biggest concern really boils down to the fact that we're assuming the first right paren we come to after a "throw" keyword in a dynamic exception specification is the terminating right paren. I think we need to use paren matching instead. It would be nice if we could use BalancedDelimiterTracker to solve this, come to think of it.

[alexfh]

clang-tidy/modernize/UseNoexceptCheck.cpp

40

Yes, tracking balanced parenthesis sequences makes sense in any case.

[hintonda]

• First cut on a simple parser for decls.

Successfully parses all the examples I've been given so far. Please
help me break it.

[hintonda]

• Added another test.

[hintonda]

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.

[hintonda]

Adjust matcher, add better error checking, and additional test cases.

We can now parse all dynamic exception specifications, but haven't
been able to develop a matcher that will find the following function
declaration without finding other nodes that aren't parsable.

void g(void (*fp)(void) throw());

[hintonda]

Btw, this version can successfully check libcxx.

[alexfh]

Let's wait for http://reviews.llvm.org/D20428

[hintonda]

Fixed matcher -- added 'unless(isImplicit())'. Thanks to Aaron Ballman for the suggestion.

[hintonda]

In D18575#435388, @alexfh wrote:

Let's wait for http://reviews.llvm.org/D20428

No worries.

[hintonda]

Actually, this will never work correctly -- in fact, raw lexing will always be problematic. Consider:

void foo()
#if !__has_feature(cxx_noexcept)

throw(std::bad_alloc)

#endif
{}

In this case we *could* figure out if __has_feature(cxx_noexcept) evaluated to true or false, but what if it was a macro instead? We can't reliably handle it without reparsing the whole thing.

I'll leave it as is for now, but once http://reviews.llvm.org/D20428 is available, I'll start a new diff using it.

[hintonda]

Please see D20693 for an alternative approach.

[alexfh]

Cleaning up my Phab dashboard. If http://reviews.llvm.org/D20428 doesn't happen and we'll have to return to this implementation, please re-add me to the reviewers.

[hintonda]

This revision is OBE.

clang-tidy/modernize/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyModernizeModule
   DeprecatedHeadersCheck.cpp
   LoopConvertCheck.cpp
   LoopConvertUtils.cpp
   MakeUniqueCheck.cpp
   ModernizeTidyModule.cpp
   PassByValueCheck.cpp
   RawStringLiteralCheck.cpp
   RedundantVoidArgCheck.cpp
   ReplaceAutoPtrCheck.cpp
   ShrinkToFitCheck.cpp
   UseAutoCheck.cpp
   UseDefaultCheck.cpp
+  UseNoexceptCheck.cpp
   UseNullptrCheck.cpp
   UseOverrideCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyReadabilityModule
   clangTidyUtils
   )

clang-tidy/modernize/ModernizeTidyModule.cpp

 #include "MakeUniqueCheck.h"
 #include "PassByValueCheck.h"
 #include "RawStringLiteralCheck.h"
 #include "RedundantVoidArgCheck.h"
 #include "ReplaceAutoPtrCheck.h"
 #include "ShrinkToFitCheck.h"
 #include "UseAutoCheck.h"
 #include "UseDefaultCheck.h"
+#include "UseNoexceptCheck.h"
 #include "UseNullptrCheck.h"
 #include "UseOverrideCheck.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace modernize {
CheckFactories.registerCheck<RawStringLiteralCheck>(
         "modernize-raw-string-literal");
     CheckFactories.registerCheck<RedundantVoidArgCheck>(
         "modernize-redundant-void-arg");
     CheckFactories.registerCheck<ReplaceAutoPtrCheck>(
         "modernize-replace-auto-ptr");
     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");
   }
 
   ClangTidyOptions getModuleOptions() override {
     ClangTidyOptions Options;
     auto &Opts = Options.CheckOptions;
     // For types whose size in bytes is above this threshold, we prefer taking a

clang-tidy/modernize/UseNoexceptCheck.h

+//===--- 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(true).
+/// \code
+///   void foo() throw();
+/// \endcode
+/// Is converted to:
+/// \code
+///   void foo() noexcept;
+/// \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 ReplacementStr;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_NOEXCEPT_H

clang-tidy/modernize/UseNoexceptCheck.cpp

+//===--- 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"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+
+// FIXME: Move to ASTMatchers.h on acceptance.
+namespace ast_matchers {
+/// \brief Matches functions that have a dynamic exception specification.
+///
+/// Given:
+/// \code
+///   void f();
+///   void g() noexcept;
+///   void h() throw();
+///   void i() throw(int);
+///   void j() throw(...);
+///   void k() noexcept(false);
+/// \endcode
+/// functionDecl(hasDynamicExceptionSpec())
+///   matches the declarations of h, i, and j, but not f, g or k.
+AST_MATCHER(FunctionDecl, hasDynamicExceptionSpec) {
+  if (const auto *FnTy = Node.getType()->getAs<FunctionProtoType>())
+    return FnTy->hasDynamicExceptionSpec();
+  return false;
+}
+}
+
+namespace tidy {
+namespace modernize {
+
+UseNoexceptCheck::UseNoexceptCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      ReplacementStr(Options.get("ReplacementString", "noexcept")) {}
+
+void UseNoexceptCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "ReplacementString", ReplacementStr);
+}
+
+void UseNoexceptCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+  Finder->addMatcher(
+      functionDecl(hasDynamicExceptionSpec()).bind("functionDecl"), this);
+}
+
+void UseNoexceptCheck::check(const MatchFinder::MatchResult &Result) {
+  SourceManager &SM = *Result.SourceManager;
+  const ASTContext &Context = *Result.Context;
+
+  const FunctionDecl *FuncDecl =
+      Result.Nodes.getNodeAs<clang::FunctionDecl>("functionDecl");
+  if (!FuncDecl)
+    return;
+
+  SourceRange Range = FuncDecl->getSourceRange();
+  if (!Range.isValid())
+    return;
+
+  SourceLocation CurrentLoc = Range.getEnd();
+  SourceLocation ReplaceStart;
+  SourceLocation ReplaceEnd;
+  std::string Replacement{ReplacementStr};
+  unsigned TokenLength{0};
+
+  SmallVector<Token, 16> Tokens{
+      lexer_utils::ParseTokens(Context, SM, CharSourceRange(Range, true))};
+  auto TokensEnd = Tokens.rend();
+  for (auto it = Tokens.rbegin(); it != TokensEnd; ++it) {
+    SourceLocation Loc = it->getLocation();
+    TokenLength = it->getLength();
+
+    // Looking for throw(), throw(<exception>[,...]), or throw(...).
+    if (it->is(tok::r_paren)) {
+      if (++it == TokensEnd)
+        return;
+      bool empty = true;
+      // Found ')', now loop till we find '('.
+      while (it->isNot(tok::l_paren)) {
+        empty = false;
+        if (++it == TokensEnd)
+          return;
+      }
+      if (++it == TokensEnd)
+        return;
+      if (StringRef(SM.getCharacterData(it->getLocation()), it->getLength()) ==
+          "throw") {
+        if (!empty) {
+          // We only support macro replacement for "throw()".
+          if (Replacement != "noexcept")
+            break;
+          Replacement = "noexcept(false)";
+        }
+        ReplaceEnd = Loc;
+        ReplaceStart = it->getLocation();
+        break;
+      }
+    } else if (++it == TokensEnd)
+      return;
+    CurrentLoc = it->getLocation();
+  }
+
+  if (ReplaceStart.isValid() && ReplaceEnd.isValid()) {
+    std::pair<FileID, unsigned> beginInfo = SM.getDecomposedLoc(ReplaceStart);
+    std::pair<FileID, unsigned> endInfo = SM.getDecomposedLoc(ReplaceEnd);
+    if (beginInfo.first != endInfo.first || beginInfo.second > endInfo.second)
+      return;
+    auto Len = endInfo.second - beginInfo.second + TokenLength;
+    diag(FuncDecl->getLocation(), "function '%0' uses dynamic exception "
+                                  "specification '%1'; use '%2' instead")
+        << FuncDecl->getNameInfo().getAsString()
+        << StringRef(SM.getCharacterData(ReplaceStart), Len) << Replacement
+        << FixItHint::CreateReplacement(
+               CharSourceRange::getTokenRange(ReplaceStart, ReplaceEnd),
+               Replacement);
+  }
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang

clang-tidy/utils/LexerUtils.h

 //===--- LexerUtils.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_UTILS_LEXER_UTILS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_LEXER_UTILS_H
 
 #include "clang/AST/ASTContext.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/SmallVector.h"
 
 namespace clang {
 namespace tidy {
 namespace lexer_utils {
 
 // Returns previous non-comment token skipping over any comment text or
 // tok::unknown if not found.
 Token getPreviousNonCommentToken(const ASTContext &Context,
                                  SourceLocation Location);
 
+SmallVector<Token, 16> ParseTokens(const ASTContext &Context,
+                                   const SourceManager &Sources,
+                                   CharSourceRange Range);
+
 } // namespace lexer_utils
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_LEXER_UTILS_H

clang-tidy/utils/LexerUtils.cpp

if (!Lexer::getRawToken(Location, Token, SourceManager,
         !Token.is(tok::comment)) {
       break;
     }
     Location = Location.getLocWithOffset(-1);
   }
   return Token;
 }
 
+SmallVector<Token, 16> ParseTokens(const ASTContext &Context,
+                                   const SourceManager &Sources,
+                                   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_utils
 } // namespace tidy
 } // namespace clang

docs/ReleaseNotes.rst

- New `modernize-deprecated-headers
   Replaces C standard library headers with their C++ alternatives.
 
 - New `modernize-raw-string-literal
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-raw-string-literal.html>`_ check
 
   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.
+
 - New `performance-faster-string-find
   <http://clang.llvm.org/extra/clang-tidy/checks/performance-faster-string-find.html>`_ check
 
   Optimize calls to ``std::string::find()`` and friends when the needle passed
   is a single character string literal.
 
 - New `performance-implicit-cast-in-loop
   <http://clang.llvm.org/extra/clang-tidy/checks/performance-implicit-cast-in-loop.html>`_ check

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

+.. 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
+``throw(false)``.  Additinally, users can also use
+:option:``ReplacementString`` to specify a macro to use instead of
+``noexcept``.  This is useful when maintaining source code that must
+compile on older compilers that don't support the ``noexcept``
+keyword.
+
+Example
+^^^^^^^
+
+.. code-block:: c++
+
+  void foo() throw() {}
+
+transforms to:
+
+.. code-block:: c++
+
+  void foo() NOEXCEPT {}
+
+if the ``ReplacementString`` option is set to ``NOEXCEPT``.

test/clang-tidy/modernize-use-noexcept-macro.cpp

+// RUN: %check_clang_tidy %s modernize-use-noexcept %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-noexcept.ReplacementString, value: 'NOEXCEPT'}]}" \
+// RUN:   -- -std=c++11
+
+#define NOEXCEPT noexcept
+
+void bar() throw() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bar' uses dynamic exception specification 'throw()'; use 'NOEXCEPT' instead [modernize-use-noexcept]
+// CHECK-FIXES: void bar() NOEXCEPT {}
+
+// Should not trigger a replacement.
+void foo() noexcept(true);
+

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

+// 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()'; use 'noexcept' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foo() noexcept;
+
+void bar() throw(...);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bar' uses dynamic exception specification 'throw(...)'; use 'noexcept(false)' instead [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)'; use 'noexcept(false)' instead [modernize-use-noexcept]
+// CHECK-FIXES: void foobar() noexcept(false)
+
+// Should not trigger a replacement.
+void titi() noexcept {}
+void toto() noexcept(true) {}