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

2016-06-03 Thread don hinton via cfe-commits
hintonda abandoned this revision.
hintonda added a comment.

This revision is OBE.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-06-03 Thread Alexander Kornienko via cfe-commits
alexfh resigned from this revision.
alexfh removed a reviewer: alexfh.
alexfh added a comment.

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.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-26 Thread don hinton via cfe-commits
hintonda added a comment.

Please see http://reviews.llvm.org/D20693 for an alternative approach.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-26 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment.

http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-23 Thread don hinton via cfe-commits
hintonda added a comment.

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.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-23 Thread don hinton via cfe-commits
hintonda added a comment.

In http://reviews.llvm.org/D18575#435388, @alexfh wrote:

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


No worries.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-23 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 58151.
hintonda added a comment.

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


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,59 @@
+// 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) {}
+
+// We can fix this one because the matcher finds the trailing throw().
+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);
+
+// FIXME: We can't fix this one -- need help developing an appropriate matcher.
+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) {}
+
+
+class Y {
+  Y() throw() = default;
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: function 'Y' uses dynamic exception specification 'throw()' [modernize-use-noexcept]
+// CHECK-FIXES: Y() noexcept = default;
+
+// We can't find this
+#define xxx X() throw() = default
+class X {
+  xxx;
+};
+
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., 

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

2016-05-20 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

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


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-19 Thread don hinton via cfe-commits
hintonda added a comment.

Btw, this version can successfully check libcxx.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-18 Thread don hinton via cfe-commits
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
+

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

2016-05-17 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57555.
hintonda added a comment.

- Added another test.


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([,...])``, 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([,...])`` or ``throw(...)`` with
+``noexcept(false)``.  Additionally, users can also use

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

2016-05-17 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57547.
hintonda added a comment.

- First cut on a simple parser for decls.

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


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,39 @@
+// 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 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([,...])``, 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([,...])`` 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 

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

2016-05-17 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:39
@@ +38,3 @@
+
+  // FIXME: Add paren matching so we can parse more complex throw statements,
+  // e.g., (examples provided by Aaron Ballman):

aaron.ballman wrote:
> alexfh wrote:
> > aaron.ballman wrote:
> > > alexfh wrote:
> > > > hintonda wrote:
> > > > > aaron.ballman wrote:
> > > > > > @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()`).
> > > > > 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?
> > > > > 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.
> > > 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.
> > 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?
> 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.
Yes, tracking balanced parenthesis sequences makes sense in any case.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-17 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:39
@@ +38,3 @@
+
+  // FIXME: Add paren matching so we can parse more complex throw statements,
+  // e.g., (examples provided by Aaron Ballman):

alexfh wrote:
> aaron.ballman wrote:
> > alexfh wrote:
> > > hintonda wrote:
> > > > aaron.ballman wrote:
> > > > > @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()`).
> > > > 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?
> > > > 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.
> > 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.
> 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?
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.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-17 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:39
@@ +38,3 @@
+
+  // FIXME: Add paren matching so we can parse more complex throw statements,
+  // e.g., (examples provided by Aaron Ballman):

aaron.ballman wrote:
> alexfh wrote:
> > hintonda wrote:
> > > aaron.ballman wrote:
> > > > @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()`).
> > > 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?
> > > 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.
> 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.
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?


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-17 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:39
@@ +38,3 @@
+
+  // FIXME: Add paren matching so we can parse more complex throw statements,
+  // e.g., (examples provided by Aaron Ballman):

alexfh wrote:
> hintonda wrote:
> > aaron.ballman wrote:
> > > @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()`).
> > 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?
> > 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.
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.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-17 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:22
@@ +21,3 @@
+namespace {
+
+StringRef makeDynamicExceptionString(const SourceManager ,

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" 


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:39
@@ +38,3 @@
+
+  // FIXME: Add paren matching so we can parse more complex throw statements,
+  // e.g., (examples provided by Aaron Ballman):

hintonda wrote:
> aaron.ballman wrote:
> > @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()`).
> 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?
> 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.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-17 Thread don hinton via cfe-commits
hintonda added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:39
@@ +38,3 @@
+
+  // FIXME: Add paren matching so we can parse more complex throw statements,
+  // e.g., (examples provided by Aaron Ballman):

aaron.ballman wrote:
> @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()`).
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?


Comment at: docs/clang-tidy/checks/modernize-use-noexcept.rst:40
@@ +39,3 @@
+``noexcept(false)`` not ``noexcept``, this check will detect, but not
+provide a FixItHint in that case.
+

aaron.ballman wrote:
> 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."
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).


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-17 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:39
@@ +38,3 @@
+
+  // FIXME: Add paren matching so we can parse more complex throw statements,
+  // e.g., (examples provided by Aaron Ballman):

@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()`).


Comment at: docs/clang-tidy/checks/modernize-use-noexcept.rst:40
@@ +39,3 @@
+``noexcept(false)`` not ``noexcept``, this check will detect, but not
+provide a FixItHint in that case.
+

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."


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-16 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57432.
hintonda added a comment.

- Address additional comments.


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,29 @@
+// 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 f(void (*fp)(void) throw()) throw(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' uses dynamic exception specification 'throw(char)' [modernize-use-noexcept]
+// CHECK-FIXES: void f(void (*fp)(void) throw()) noexcept(false);
+
+// FIXME: We can't match parameters yet.
+void g(void (*fp)(void) throw());
+
+// 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([,...])``, 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([,...])`` 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 

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

2016-05-16 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.


Comment at: test/clang-tidy/modernize-use-noexcept-macro.cpp:11
@@ +10,3 @@
+
+// Should not trigger a FixItHint
+class A {};

hintonda wrote:
> aaron.ballman wrote:
> > I may have missed some context in the discussion, but why shouldn't this 
> > trigger a FixItHint?
> 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.
> 
Ah, thank you for the context! That information should probably go in the user 
facing documentation somewhere.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-16 Thread don hinton via cfe-commits
hintonda added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:46
@@ +45,3 @@
+   const SmallVector ) {
+  // Find throw token -- it's a keyword, so there can't be more than one.  
Also,
+  // it should be near the end of the declaration, so search from the end.

aaron.ballman wrote:
> hintonda wrote:
> > aaron.ballman wrote:
> > > Pathologically terrible counter-case: `void func() throw(decltype(throw 
> > > 12) *)`
> > Good point, looks like I need a full fledged parser to catch 100% or the 
> > cases -- or we could ignore these corner cases.
> 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));`
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.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:49
@@ +48,3 @@
+  int TokenIndex;
+  for (TokenIndex = Tokens.size() - 1; TokenIndex != -1; --TokenIndex) {
+if (Tokens[TokenIndex].is(tok::kw_throw))

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


Comment at: test/clang-tidy/modernize-use-noexcept-macro.cpp:11
@@ +10,3 @@
+
+// Should not trigger a FixItHint
+class A {};

aaron.ballman wrote:
> I may have missed some context in the discussion, but why shouldn't this 
> trigger a FixItHint?
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.



http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-16 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:21
@@ +20,3 @@
+
+static StringRef
+makeDynamicExceptionString(const SourceManager ,

hintonda wrote:
> aaron.ballman wrote:
> > Instead of a bunch of static functions, would an unnamed namespace make 
> > more sense?
> Just following the pattern established in other checkers so as to minimize 
> the number of changes, but will change to namespace.
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).


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:46
@@ +45,3 @@
+   const SmallVector ) {
+  // Find throw token -- it's a keyword, so there can't be more than one.  
Also,
+  // it should be near the end of the declaration, so search from the end.

hintonda wrote:
> aaron.ballman wrote:
> > Pathologically terrible counter-case: `void func() throw(decltype(throw 12) 
> > *)`
> Good point, looks like I need a full fledged parser to catch 100% or the 
> cases -- or we could ignore these corner cases.
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));`


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-16 Thread don hinton via cfe-commits
hintonda added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:21
@@ +20,3 @@
+
+static StringRef
+makeDynamicExceptionString(const SourceManager ,

aaron.ballman wrote:
> Instead of a bunch of static functions, would an unnamed namespace make more 
> sense?
Just following the pattern established in other checkers so as to minimize the 
number of changes, but will change to namespace.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:34
@@ +33,3 @@
+
+static CharSourceRange makeMoveRange(const SourceManager ,
+ const LangOptions ,

aaron.ballman wrote:
> 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?
Was a bit more involved, but last round of changes simplified it.  Will 
simplify even more.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:46
@@ +45,3 @@
+   const SmallVector ) {
+  // Find throw token -- it's a keyword, so there can't be more than one.  
Also,
+  // it should be near the end of the declaration, so search from the end.

aaron.ballman wrote:
> Pathologically terrible counter-case: `void func() throw(decltype(throw 12) 
> *)`
Good point, looks like I need a full fledged parser to catch 100% or the cases 
-- or we could ignore these corner cases.


Comment at: clang-tidy/modernize/UseNoexceptCheck.h:22
@@ +21,3 @@
+/// \brief Replace dynamic exception specifications, with
+/// `noexcept` (or user-defined macro) or `noexcept(true)`.
+/// \code

aaron.ballman wrote:
> I think this comment means `noexcept(false)` instead? Or is there a reason to 
> replace with `noexcept(true)` instead of just `noexcept`?
Typo, should just be noexcept.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-16 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:21
@@ +20,3 @@
+
+static StringRef
+makeDynamicExceptionString(const SourceManager ,

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:34
@@ +33,3 @@
+
+static CharSourceRange makeMoveRange(const SourceManager ,
+ const LangOptions ,

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?


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:46
@@ +45,3 @@
+   const SmallVector ) {
+  // Find throw token -- it's a keyword, so there can't be more than one.  
Also,
+  // it should be near the end of the declaration, so search from the end.

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:49
@@ +48,3 @@
+  int TokenIndex;
+  for (TokenIndex = Tokens.size() - 1; TokenIndex != -1; --TokenIndex) {
+if (Tokens[TokenIndex].is(tok::kw_throw))

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:100
@@ +99,3 @@
+
+  auto FileMoveRange = createReplacementRange(SM, getLangOpts(), Tokens);
+

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:108
@@ +107,3 @@
+
+  diag(FuncDecl->getLocation(), "function '%0' uses dynamic exception "
+"specification '%1'")

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.h:22
@@ +21,3 @@
+/// \brief Replace dynamic exception specifications, with
+/// `noexcept` (or user-defined macro) or `noexcept(true)`.
+/// \code

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


Comment at: test/clang-tidy/modernize-use-noexcept-macro.cpp:11
@@ +10,3 @@
+
+// Should not trigger a FixItHint
+class A {};

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


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-16 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57357.
hintonda added a comment.

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


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,22 @@
+// 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)
+
+// 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,18 @@
+// 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()' [modernize-use-noexcept]
+// CHECK-FIXES: void bar() NOEXCEPT {}
+
+// Should not trigger a FixItHint
+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,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., ``throw()``,
+``throw([,...])``, 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([,...])`` 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.
+
+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
+  `_ check
+
+  Replaces dynamic exception specifications with ``noexcept`` or a user defined macro.
+
 - New `performance-faster-string-find
   `_ 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 

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

2016-05-15 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57315.
hintonda added a comment.

- Fix some function names.


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,22 @@
+// 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)
+
+// 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,18 @@
+// 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()' [modernize-use-noexcept]
+// CHECK-FIXES: void bar() NOEXCEPT {}
+
+// Should not trigger a FixItHint
+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,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., ``throw()``,
+``throw([,...])``, 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([,...])`` 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.
+
+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
+  `_ check
+
+  Replaces dynamic exception specifications with ``noexcept`` or a user defined macro.
+
 - New `performance-faster-string-find
   `_ 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 ,
  SourceLocation Location);
 
+SmallVector 

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

2016-05-15 Thread Etienne Bergeron via cfe-commits
etienneb added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:22
@@ +21,3 @@
+static StringRef
+MakeDynamicExceptionString(const SourceManager ,
+   const CharSourceRange ) {

coding style:
MakeDynamicExceptionString -> makeDynamicExceptionString


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:34
@@ +33,3 @@
+
+static CharSourceRange MakeMoveRange(const SourceManager ,
+ const LangOptions ,

MakeMoveRange -> makeMoveRange

(and all other occurrences below)


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-15 Thread don hinton via cfe-commits
hintonda added a comment.

I think this is now ready for review.

Thanks again for your patience...


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-15 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57305.
hintonda added a comment.

- A bit more refactoring and cleanup.


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,22 @@
+// 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)
+
+// 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,18 @@
+// 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()' [modernize-use-noexcept]
+// CHECK-FIXES: void bar() NOEXCEPT {}
+
+// Should not trigger a FixItHint
+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,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., ``throw()``,
+``throw([,...])``, 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([,...])`` 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.
+
+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
+  `_ check
+
+  Replaces dynamic exception specifications with ``noexcept`` or a user defined macro.
+
 - New `performance-faster-string-find
   `_ 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 ,
  SourceLocation Location);
 
+SmallVector 

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

2016-05-15 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57301.
hintonda added a comment.

- Added asserts for indexes into Tokens array.


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,22 @@
+// 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) {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,18 @@
+// 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 FixItHint
+class A {};
+class B {};
+void foobar() throw(A, B);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function ''foobar'' uses dynamic exception specification 'throw(A, B)'; use 'noexcept(false)' instead [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,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., ``throw()``,
+``throw([,...])``, 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([,...])`` 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.
+
+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
+  `_ check
+
+  Replaces dynamic exception specifications with ``noexcept`` or a user defined macro.
+
 - New `performance-faster-string-find
   `_ 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 

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

2016-05-15 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57299.
hintonda added a comment.

- Make helper functions static.


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,22 @@
+// 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) {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,18 @@
+// 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 FixItHint
+class A {};
+class B {};
+void foobar() throw(A, B);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function ''foobar'' uses dynamic exception specification 'throw(A, B)'; use 'noexcept(false)' instead [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,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., ``throw()``,
+``throw([,...])``, 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([,...])`` 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.
+
+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
+  `_ check
+
+  Replaces dynamic exception specifications with ``noexcept`` or a user defined macro.
+
 - New `performance-faster-string-find
   `_ 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 

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

2016-05-15 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57298.
hintonda added a comment.

- Complete refactor


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,22 @@
+// 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) {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,18 @@
+// 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 FixItHint
+class A {};
+class B {};
+void foobar() throw(A, B);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function ''foobar'' uses dynamic exception specification 'throw(A, B)'; use 'noexcept(false)' instead [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,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., ``throw()``,
+``throw([,...])``, 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([,...])`` 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.
+
+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
+  `_ check
+
+  Replaces dynamic exception specifications with ``noexcept`` or a user defined macro.
+
 - New `performance-faster-string-find
   `_ 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 

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

2016-05-14 Thread don hinton via cfe-commits
hintonda added a comment.

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.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-14 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:37
@@ +36,3 @@
+bool UseNoexceptCheck::checkHelper(const MatchFinder::MatchResult ,
+   const SourceRange ,
+   CharSourceRange ,

> 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 Tokens)`, for 
example.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:121
@@ +120,3 @@
+FileMoveRange.getEnd()),
+ Replacement);
+}

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.h:44-45
@@ +43,4 @@
+   unsigned );
+  const std::string ReplacementStr;
+  StringRef Replacement;
+};

hintonda wrote:
> aaron.ballman wrote:
> > 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.
> 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.
The `StringRef Replacement;` should not be a member, since it's just one of the 
results of the `checkHelper` method.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-13 Thread don hinton via cfe-commits
hintonda added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.h:44-45
@@ +43,4 @@
+   unsigned );
+  const std::string ReplacementStr;
+  StringRef Replacement;
+};

aaron.ballman wrote:
> 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.
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.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-13 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.h:44-45
@@ +43,4 @@
+   unsigned );
+  const std::string ReplacementStr;
+  StringRef Replacement;
+};

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.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-12 Thread don hinton via cfe-commits
hintonda added inline comments.


Comment at: clang-tidy/utils/LexerUtils.cpp:38
@@ -37,1 +37,3 @@
 
+SmallVector ParseTokens(const ASTContext ,
+   const SourceManager ,

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. 


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-12 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57103.
hintonda added a comment.

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


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,22 @@
+// 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) {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,13 @@
+// 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);
+
Index: docs/clang-tidy/checks/modernize-use-noexcept.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-noexcept.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., ``throw()``,
+``throw([,...])``, 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([,...])`` 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.
+
+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
+  `_ check
+
+  Replaces dynamic exception specifications with ``noexcept`` or a user defined macro.
+
 - New `performance-faster-string-find
   `_ 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 ,
  SourceLocation Location);
 
+SmallVector ParseTokens(const ASTContext ,
+   const SourceManager ,
+

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

2016-05-12 Thread don hinton via cfe-commits
hintonda added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:99
@@ +98,3 @@
+  if (Tok.is(tok::l_paren)) {
+Tok = getPreviousNonCommentToken(Context, Tok.getLocation());
+found = std::string(GetText(Tok, SM)) + "(true)";

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.


Comment at: clang-tidy/utils/LexerUtils.cpp:38
@@ -37,1 +37,3 @@
 
+SmallVector ParseTokens(const ASTContext ,
+   const SourceManager ,

etienneb wrote:
> If you call ParseTokens, it will parse tokens until it reach a 
> semicolon/lbrace? exact?
> 
Yes, but again, I'm not sure what you are asking me to do/change.

Are you asking me to change something?


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-12 Thread Etienne Bergeron via cfe-commits
etienneb added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:98
@@ +97,3 @@
+  const auto *FuncDecl = Result.Nodes.getNodeAs("functionDecl");
+  if (!FuncDecl)
+return;

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


Comment at: clang-tidy/utils/LexerUtils.cpp:38
@@ -37,1 +37,3 @@
 
+SmallVector ParseTokens(const ASTContext ,
+   const SourceManager ,

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



http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-12 Thread 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 ) {
+  auto FuncDecl = Result.Nodes.getNodeAs("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 ParseTokens(const ASTContext ,
+   const SourceManager ,

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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-12 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57097.
hintonda added a comment.

- Addressed a few comments for clarity/readability.


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,22 @@
+// 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) {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,13 @@
+// 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);
+
Index: docs/clang-tidy/checks/modernize-use-noexcept.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-noexcept.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., ``throw()``,
+``throw([,...])``, 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([,...])`` 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.
+
+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
+  `_ check
+
+  Replaces dynamic exception specifications with ``noexcept`` or a user defined macro.
+
 - New `performance-faster-string-find
   `_ 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 ,
  SourceLocation Location);
 
+SmallVector ParseTokens(const ASTContext ,
+   const SourceManager ,
+   

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

2016-05-12 Thread Etienne Bergeron via cfe-commits
etienneb added a subscriber: etienneb.
etienneb added a comment.

drive by



Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:40
@@ +39,3 @@
+   unsigned ) {
+  SourceManager  = *Result.SourceManager;
+  const ASTContext  = *Result.Context;

nit: const


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:97
@@ +96,3 @@
+void UseNoexceptCheck::check(const MatchFinder::MatchResult ) {
+  auto FuncDecl = Result.Nodes.getNodeAs("functionDecl");
+  if (!FuncDecl)

nit:  const auto FuncDecl* 


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:114
@@ +113,3 @@
+"specification '%1'; use '%2' instead")
+  << FuncDecl->getNameInfo().getAsString()
+  << StringRef(

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

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

IIRC, there is a automatic conversion for the name.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:120
@@ +119,3 @@
+  << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(FileMoveRange.getBegin(),
+FileMoveRange.getEnd()),

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

```
 CharSourceRange FileMoveRange;
```


Comment at: clang-tidy/utils/LexerUtils.cpp:38
@@ -37,1 +37,3 @@
 
+SmallVector ParseTokens(const ASTContext ,
+   const SourceManager ,

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


Comment at: test/clang-tidy/modernize-use-noexcept.cpp:4
@@ +3,3 @@
+
+class A{};
+class B{};

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


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-12 Thread don hinton via cfe-commits
hintonda added inline comments.


Comment at: docs/clang-tidy/checks/modernize-use-noexcept.rst:50
@@ +49,2 @@
+
+if the :option:`ReplacementString` option is set to `NOEXCEPT`.

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


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-12 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57080.
hintonda added a comment.

- Add :option: prefix for clarity.


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,22 @@
+// 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) {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,13 @@
+// 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);
+
Index: docs/clang-tidy/checks/modernize-use-noexcept.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-noexcept.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., ``throw()``,
+``throw([,...])``, 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([,...])`` 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.
+
+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
+  `_ check
+
+  Replaces dynamic exception specifications with ``noexcept`` or a user defined macro.
+
 - New `performance-faster-string-find
   `_ 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 ,
  SourceLocation Location);
 
+SmallVector ParseTokens(const ASTContext ,
+   const SourceManager ,
+   

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

2016-05-12 Thread Eugene Zelenko via cfe-commits
Eugene.Zelenko added inline comments.


Comment at: docs/clang-tidy/checks/modernize-use-noexcept.rst:50
@@ +49,2 @@
+
+if the `ReplacementString` option is set to `NOEXCEPT`.

Actually :option: still need to prepend `ReplacementString`.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-12 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57078.
hintonda added a comment.

- Fix quote problem in docs.


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,22 @@
+// 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) {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,13 @@
+// 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);
+
Index: docs/clang-tidy/checks/modernize-use-noexcept.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-noexcept.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., ``throw()``,
+``throw([,...])``, 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([,...])`` 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.
+
+Example
+^^^
+
+.. code-block:: c++
+
+  void foo() throw() {}
+
+transforms to:
+
+.. code-block:: c++
+
+  void foo() NOEXCEPT {}
+
+if the `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
+  `_ check
+
+  Replaces dynamic exception specifications with ``noexcept`` or a user defined macro.
+
 - New `performance-faster-string-find
   `_ 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 ,
  SourceLocation Location);
 
+SmallVector ParseTokens(const ASTContext ,
+   const SourceManager ,
+   CharSourceRange Range);

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

2016-05-12 Thread don hinton via cfe-commits
hintonda added inline comments.


Comment at: docs/clang-tidy/checks/modernize-use-noexcept.rst:32
@@ +31,3 @@
+``noexcept(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

Eugene.Zelenko wrote:
> hintonda wrote:
> > Eugene.Zelenko wrote:
> > > Sorry, missed last time. Please use ` for check option and its value(s). 
> > > Same for last line in file.
> > I'm not sure I understand what you want me to change.  Did you want me to 
> > append that phrase, or replace something?
> 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.
Ah, got it.  Sorry for being so dense...  Will checkin shortly...


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-12 Thread Eugene Zelenko via cfe-commits
Eugene.Zelenko added inline comments.


Comment at: docs/clang-tidy/checks/modernize-use-noexcept.rst:32
@@ +31,3 @@
+``noexcept(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

hintonda wrote:
> Eugene.Zelenko wrote:
> > Sorry, missed last time. Please use ` for check option and its value(s). 
> > Same for last line in file.
> I'm not sure I understand what you want me to change.  Did you want me to 
> append that phrase, or replace something?
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.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-12 Thread don hinton via cfe-commits
hintonda added inline comments.


Comment at: docs/clang-tidy/checks/modernize-use-noexcept.rst:32
@@ +31,3 @@
+``noexcept(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

Eugene.Zelenko wrote:
> Sorry, missed last time. Please use ` for check option and its value(s). Same 
> for last line in file.
I'm not sure I understand what you want me to change.  Did you want me to 
append that phrase, or replace something?


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-12 Thread Eugene Zelenko via cfe-commits
Eugene.Zelenko added inline comments.


Comment at: docs/clang-tidy/checks/modernize-use-noexcept.rst:32
@@ +31,3 @@
+``noexcept(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

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


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-12 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57070.
hintonda added a comment.

- Added 's around keywords in docs.


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,22 @@
+// 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) {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,13 @@
+// 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);
+
Index: docs/clang-tidy/checks/modernize-use-noexcept.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-noexcept.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., ``throw()``,
+``throw([,...])``, 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([,...])`` or ``throw(...)`` with
+``noexcept(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
+be compiled with 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``.
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
+  `_ check
+
+  Replaces dynamic exception specifications with ``noexcept`` or a user defined macro.
+
 - New `performance-faster-string-find
   `_ 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 ,
  SourceLocation Location);
 
+SmallVector ParseTokens(const ASTContext ,
+   const SourceManager ,
+   

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

2016-05-12 Thread Eugene Zelenko via cfe-commits
Eugene.Zelenko added inline comments.


Comment at: docs/ReleaseNotes.rst:197
@@ +196,3 @@
+
+  Replaces dynamic exception specifications with noexcept.
+

Please highlight noexcept with ``.


Comment at: docs/clang-tidy/checks/modernize-use-noexcept.rst:6
@@ +5,3 @@
+
+The check converts dynamic exception specifications, e.g., throw(),
+throw([,...]), or throw(...) to noexcept, noexcept(false),

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


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-12 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 57065.
hintonda added a comment.

- Renamed private helper method


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,22 @@
+// 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) {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,13 @@
+// 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);
+
Index: docs/clang-tidy/checks/modernize-use-noexcept.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-noexcept.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., throw(),
+throw([,...]), 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([,...])`` or ``throw(...)`` with
+``noexcept(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
+be compiled with 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``.
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
+  `_ check
+
+  Replaces dynamic exception specifications with noexcept.
+
 - New `performance-faster-string-find
   `_ 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 ,
  SourceLocation Location);
 
+SmallVector ParseTokens(const ASTContext ,
+   const SourceManager ,
+   CharSourceRange Range);
+
 } // namespace lexer
 } // namespace 

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

2016-05-11 Thread don hinton via cfe-commits
hintonda added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:36
@@ +35,3 @@
+
+bool UseNoexceptCheck::helper(const MatchFinder::MatchResult ,
+  const SourceRange ,

alexfh wrote:
> 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` or some other way to inform the caller that there's no 
> action required.
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?


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-11 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:36
@@ +35,3 @@
+
+bool UseNoexceptCheck::helper(const MatchFinder::MatchResult ,
+  const SourceRange ,

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` or 
some other way to inform the caller that there's no action required.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-11 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 56984.
hintonda added a comment.

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


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,22 @@
+// 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) {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,13 @@
+// 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);
+
Index: docs/clang-tidy/checks/modernize-use-noexcept.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-noexcept.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., throw(),
+throw([,...]), 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([,...])`` or ``throw(...)`` with
+``noexcept(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
+be compiled with 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``.
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
+  `_ check
+
+  Replaces dynamic exception specifications with noexcept.
+
 - New `performance-faster-string-find
   `_ 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 ,
  SourceLocation Location);
 
+SmallVector ParseTokens(const ASTContext ,
+   const SourceManager ,
+   

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

2016-05-11 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:39
@@ +38,3 @@
+
+  const FunctionDecl *FuncDecl =
+  Result.Nodes.getNodeAs("functionDecl");

s/FunctionDecl/auto/


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:40
@@ +39,3 @@
+  const FunctionDecl *FuncDecl =
+  Result.Nodes.getNodeAs("functionDecl");
+  if (!FuncDecl)

s/clang:://


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:49-50
@@ +48,4 @@
+  SourceLocation CurrentLoc = Range.getEnd();
+  SourceLocation ReplaceStart;
+  SourceLocation ReplaceEnd;
+  std::string Replacement = ReplacementStr;

These two seem to represent a `SourceRange`.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:51
@@ +50,3 @@
+  SourceLocation ReplaceEnd;
+  std::string Replacement = ReplacementStr;
+  unsigned TokenLength = 0;

s/std::string/StringRef/


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:54-90
@@ +53,39 @@
+
+  SmallVector Tokens =
+  utils::lexer::ParseTokens(Context, SM, CharSourceRange(Range, true));
+  auto TokensEnd = Tokens.rend();
+  for (auto I = Tokens.rbegin(); I != TokensEnd; ++I) {
+SourceLocation Loc = I->getLocation();
+TokenLength = I->getLength();
+
+// Looking for throw(), throw([,...]), or throw(...).
+if (I->is(tok::r_paren)) {
+  if (++I == TokensEnd)
+return;
+  bool Empty = true;
+  // Found ')', now loop till we find '('.
+  while (I->isNot(tok::l_paren)) {
+Empty = false;
+if (++I == TokensEnd)
+  return;
+  }
+  if (++I == TokensEnd)
+return;
+  if (StringRef(SM.getCharacterData(I->getLocation()), I->getLength()) ==
+  "throw") {
+if (!Empty) {
+  // We only support macro replacement for "throw()".
+  if (Replacement != "noexcept")
+break;
+  Replacement = "noexcept(false)";
+}
+ReplaceEnd = Loc;
+ReplaceStart = I->getLocation();
+break;
+  }
+} else if (++I == TokensEnd) {
+  return;
+}
+CurrentLoc = I->getLocation();
+  }
+

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:93
@@ +92,3 @@
+  if (ReplaceStart.isValid() && ReplaceEnd.isValid()) {
+std::pair BeginInfo = SM.getDecomposedLoc(ReplaceStart);
+std::pair EndInfo = SM.getDecomposedLoc(ReplaceEnd);

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


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-05-08 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 56521.
hintonda added a comment.

- Fix typo in docs.


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,22 @@
+// 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) {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,13 @@
+// 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);
+
Index: docs/clang-tidy/checks/modernize-use-noexcept.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-noexcept.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., throw(),
+throw([,...]), 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([,...])`` or ``throw(...)`` with
+``noexcept(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
+be compiled with 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``.
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
+  `_ check
+
+  Replaces dynamic exception specifications with noexcept.
+
 - New `performance-faster-string-find
   `_ 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 ,
  SourceLocation Location);
 
+SmallVector ParseTokens(const ASTContext ,
+   const SourceManager ,
+   CharSourceRange Range);
+
 } // namespace lexer
 } // namespace utils
 } // 

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

2016-05-07 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 56511.
hintonda added a comment.

Addressed additional comments.


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,22 @@
+// 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) {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,13 @@
+// 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);
+
Index: docs/clang-tidy/checks/modernize-use-noexcept.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-noexcept.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., throw(),
+throw([,...]), 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([,...])`` 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``.
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
+  `_ check
+
+  Replaces dynamic exception specifications with noexcept.
+
 - New `performance-faster-string-find
   `_ 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 ,
  SourceLocation Location);
 
+SmallVector ParseTokens(const ASTContext ,
+   const SourceManager ,
+   CharSourceRange Range);
+
 } // namespace lexer
 } // namespace utils
 } 

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

2016-04-18 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:18
@@ +17,3 @@
+
+// FIXME: Move to ASTMatchers.h on acceptance.
+namespace ast_matchers {

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:74
@@ +73,3 @@
+  SourceLocation ReplaceEnd;
+  std::string Replacement{ReplacementStr};
+  unsigned TokenLength{0};

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.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:80
@@ +79,3 @@
+  auto TokensEnd = Tokens.rend();
+  for (auto it = Tokens.rbegin(); it != TokensEnd; ++it) {
+SourceLocation Loc = it->getLocation();

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:109
@@ +108,3 @@
+  }
+} else if (++it == TokensEnd)
+  return;

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:115
@@ +114,3 @@
+  if (ReplaceStart.isValid() && ReplaceEnd.isValid()) {
+std::pair beginInfo = SM.getDecomposedLoc(ReplaceStart);
+std::pair endInfo = SM.getDecomposedLoc(ReplaceEnd);

CamelCase


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:117
@@ +116,3 @@
+std::pair endInfo = SM.getDecomposedLoc(ReplaceEnd);
+if (beginInfo.first != endInfo.first || beginInfo.second > endInfo.second)
+  return;

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:122
@@ +121,3 @@
+  "specification '%1'; use '%2' instead")
+<< FuncDecl->getNameInfo().getAsString()
+<< StringRef(SM.getCharacterData(ReplaceStart), Len) << Replacement

Just `FuncDecl` should be enough.


Comment at: clang-tidy/modernize/UseNoexceptCheck.h:22
@@ +21,3 @@
+/// \brief Replace dynamic exception specifications, with
+/// noexcept (or user-defined macro) or noexcept(true).
+/// \code

Please enclose inline code snippets in backquotes.


Comment at: docs/clang-tidy/checks/modernize-use-noexcept.rst:32
@@ +31,3 @@
+``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

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


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-04-17 Thread don hinton via cfe-commits
hintonda marked 9 inline comments as done.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:62
@@ +61,3 @@
+
+  const FunctionDecl *FuncDecl =
+  Result.Nodes.getNodeAs("functionDecl");

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:132
@@ +131,2 @@
+} // namespace tidy
+} // namespace clang

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.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-04-17 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 54022.
hintonda added a comment.

Addressed most comments.


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/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,22 @@
+// 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) {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,13 @@
+// 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);
+
Index: docs/clang-tidy/checks/modernize-use-noexcept.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-noexcept.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., throw(),
+throw([,...]), 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([,...])`` 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``.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -146,6 +146,11 @@
   Selectively replaces string literals containing escaped characters with raw
   string literals.
 
+- New `modernize-use-noexcept
+  `_ check
+
+  Replaces dynamic exception specifications with noexcept.
+
 - New `performance-faster-string-find
   `_ 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 {
@@ -22,6 +23,10 @@
 Token getPreviousNonCommentToken(const ASTContext ,
  SourceLocation Location);
 
+SmallVector ParseTokens(const ASTContext ,
+   const SourceManager ,
+   CharSourceRange Range);
+
 } // namespace lexer_utils
 } // namespace tidy
 } // namespace clang
Index: 

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

2016-04-05 Thread don hinton via cfe-commits
hintonda added a comment.

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


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-04-05 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:16
@@ +15,3 @@
+
+// FIXME: Should this be moved to ASTMatchers.h?
+namespace ast_matchers {

alexfh wrote:
> Yes, it might make sense to move it to ASTMatchers.h.
Yes, please (along with tests and documentation, etc).


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:94
@@ +93,3 @@
+
+  const FunctionDecl *FuncDecl =
+  Result.Nodes.getNodeAs("functionDecl");

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:148
@@ +147,3 @@
+CurrentLoc = Tok.getLocation();
+  }
+

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.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-04-01 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:16
@@ +15,3 @@
+
+// FIXME: Should this be moved to ASTMatchers.h?
+namespace ast_matchers {

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:31
@@ +30,3 @@
+///   matches the declarations of h, i, and j, but not f, g or k.
+AST_MATCHER(FunctionDecl, isDynamicException) {
+  const auto *FnTy = Node.getType()->getAs();

Please rename the matcher to `hasDynamicExceptionSpec()`.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:35
@@ +34,3 @@
+return false;
+  return isDynamicExceptionSpec(FnTy->getExceptionSpecType());
+}

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:61
@@ +60,3 @@
+  CurrentLoc =
+  Lexer::GetBeginningOfToken(CurrentLoc, SM, Context.getLangOpts());
+  if (!Lexer::getRawToken(CurrentLoc, Tok, SM, Context.getLangOpts()) &&

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}.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:84
@@ +83,3 @@
+  Finder->addMatcher(
+  ast_matchers::functionDecl(ast_matchers::isDynamicException())
+  .bind("functionDecl"),

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:110
@@ +109,3 @@
+  SimpleReverseLexer Lexer{Context, SM, BeginLoc, CurrentLoc};
+  Token  = Lexer.getToken();
+  unsigned TokenLength{0};

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:123
@@ +122,3 @@
+return;
+  // if (!isBefore(BeginLoc, Tok.getLocation())) return;
+  bool empty = true;

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:156
@@ +155,3 @@
+auto Len = endInfo.second - beginInfo.second + TokenLength;
+diag(FuncDecl->getLocation(), "function '%0': Replace dynamic exception "
+  "specifications '%1' with '%2'")

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".


Comment at: clang-tidy/modernize/UseNoexceptCheck.h:35
@@ +34,3 @@
+public:
+  UseNoexceptCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context),

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.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-03-31 Thread don hinton via cfe-commits
hintonda added a comment.

Addressed comments and believe it's ready for review.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-03-31 Thread don hinton via cfe-commits
hintonda marked 8 inline comments as done.
hintonda added a comment.

http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-03-31 Thread don hinton via cfe-commits
hintonda updated this revision to Diff 52322.
hintonda added a comment.

Address comments and refactor.


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
  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,22 @@
+// 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': Replace dynamic exception specifications 'throw()' with 'noexcept' [modernize-use-noexcept]
+// CHECK-FIXES: void foo() noexcept;
+
+void bar() throw(...);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bar': Replace dynamic exception specifications 'throw(...)' with 'noexcept(false)' [modernize-use-noexcept]
+// CHECK-FIXES: void bar() noexcept(false);
+
+void foobar() throw(A, B)
+{}
+// CHECK-MESSAGES: :[[@LINE-2]]:6: warning: function 'foobar': Replace dynamic exception specifications 'throw(A, B)' with 'noexcept(false)' [modernize-use-noexcept]
+// CHECK-FIXES: void foobar() 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,13 @@
+// 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': Replace dynamic exception specifications 'throw()' with 'NOEXCEPT' [modernize-use-noexcept]
+// CHECK-FIXES: void bar() 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,50 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts dynamic exception specifications, e.g., throw(),
+throw([,...]), 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([,...])`` 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``.
Index: clang-tidy/modernize/UseNoexceptCheck.h
===
--- /dev/null
+++ clang-tidy/modernize/UseNoexceptCheck.h
@@ -0,0 +1,53 @@
+//===--- 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;
+
+/// \brief Replace dynamic exception specifications, with
+/// noexcept[({true|false})] or a user defined macro.
+/// \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)
+  : ClangTidyCheck(Name, Context),
+ReplacementStr(Options.get("ReplacementString", "noexcept")) {}
+
+  void 

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

2016-03-30 Thread don hinton via cfe-commits
hintonda added a comment.

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



Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:24
@@ +23,3 @@
+   char delimiter) {
+  SmallVector Candidates;
+  AllStrings.split(Candidates, ',');

aaron.ballman wrote:
> Why 5?
No particular reason -- copied basic implementation from 
utils::parseHeaderFileExtensions() which did something similar.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:33
@@ +32,3 @@
+
+using namespace lexer_utils;
+

aaron.ballman wrote:
> This should not be at file scope; if it really clarifies the code, it should 
> be at function scope where needed.
Will remove/refactor -- was just following the examples I found in other 
checkers.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:83
@@ +82,3 @@
+BeforeThanCompare isBefore(SM);
+while (isBefore(BeginLoc, CurrentLoc)) {
+  SourceLocation Loc = Tok.getLocation();

aaron.ballman wrote:
> This while loop could use some comments to explain what it is trying to do. 
> As best I can tell, this appears to be looking purely at the text the user 
> wrote to try to determine whether there is a `throw()` or a `noexcept(true)`, 
> but that can be done more clearly with  FunctionType::getExceptionSpecType().
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.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-03-30 Thread Eugene Zelenko via cfe-commits
Eugene.Zelenko added a comment.

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


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-03-30 Thread Aaron Ballman via cfe-commits
aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.

I *really* like this as a modernize check, especially since (throwing) dynamic 
exception specifications may be removed in C++1z. However, I think this check 
doesn't do enough yet. If dynamic exception specifications are removed in the 
next standard, `throw()` is likely to remain for backwards compatibility since 
it does map directly to `noexcept`. I think this check really needs to convert 
`throw(type)` and `throw(...)` into `noexcept(false)` as well. Further, I 
prefer not to touch `noexcept(true)` as part of this check, since that really 
isn't modernizing anything.

Thank you for working on this!



Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:24
@@ +23,3 @@
+   char delimiter) {
+  SmallVector Candidates;
+  AllStrings.split(Candidates, ',');

Why 5?


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:33
@@ +32,3 @@
+
+using namespace lexer_utils;
+

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:65
@@ +64,3 @@
+
+  if (const FunctionDecl *FuncDecl =
+  Result.Nodes.getNodeAs("functionDecl")) {

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


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:83
@@ +82,3 @@
+BeforeThanCompare isBefore(SM);
+while (isBefore(BeginLoc, CurrentLoc)) {
+  SourceLocation Loc = Tok.getLocation();

This while loop could use some comments to explain what it is trying to do. As 
best I can tell, this appears to be looking purely at the text the user wrote 
to try to determine whether there is a `throw()` or a `noexcept(true)`, but 
that can be done more clearly with  FunctionType::getExceptionSpecType().


Comment at: clang-tidy/modernize/UseNoexceptCheck.h:21
@@ +20,3 @@
+
+/// \brief Replace noexcept specifications, or macros, with noexcept or a user 
defined macro. 
+/// \code

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


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-03-30 Thread don hinton via cfe-commits
hintonda added a comment.

Will do, but that will have to be done in it's own patch.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-03-29 Thread Eugene Zelenko via cfe-commits
Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko added a comment.

Please add information about this check in docs/ReleaseNotes.rst.


http://reviews.llvm.org/D18575



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-03-29 Thread don hinton via cfe-commits
hintonda created this revision.
hintonda added a reviewer: alexfh.
hintonda added a subscriber: cfe-commits.

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

It also allows users to replace an arbitrary list of macros with
either noexcept or a user defined macro -- which is useful when
supporting older compilers.

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
  docs/clang-tidy/checks/list.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,16 @@
+// RUN: %check_clang_tidy %s modernize-use-noexcept %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-noexcept.CandidateStrings, value: 'NOTHROW,,,'}]}" \
+// RUN:   -- -std=c++11
+
+void foo() throw();
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'foo': replacing noexcept specification 'throw()' with 'noexcept' [modernize-use-noexcept]
+// CHECK-FIXES: void foo() noexcept;
+
+void bar() throw() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bar': replacing noexcept specification 'throw()' with 'noexcept' [modernize-use-noexcept]
+// CHECK-FIXES: void bar() noexcept {}
+
+#define NOTHROW throw()
+void foobar() NOTHROW {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'foobar': replacing noexcept specification 'NOTHROW' with 'noexcept' [modernize-use-noexcept]
+// CHECK-FIXES: void foobar() noexcept {}
Index: test/clang-tidy/modernize-use-noexcept-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-noexcept-macro.cpp
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy %s modernize-use-noexcept %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-noexcept.CandidateStrings, value: ' NOTHROW , , ,'}, \
+// RUN:{key: modernize-use-noexcept.ReplacementString, value: 'NOEXCEPT'}]}" \
+// RUN:   -- -std=c++11
+
+#define NOEXCEPT noexcept
+
+void foo() noexcept(true);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'foo': replacing noexcept specification 'noexcept(true)' with 'NOEXCEPT' [modernize-use-noexcept]
+// CHECK-FIXES: void foo() NOEXCEPT;
+
+void bar() throw() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bar': replacing noexcept specification 'throw()' with 'NOEXCEPT' [modernize-use-noexcept]
+// CHECK-FIXES: void bar() NOEXCEPT {}
+
+#define NOTHROW throw()
+void foobar() NOTHROW {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'foobar': replacing noexcept specification 'NOTHROW' with 'NOEXCEPT' [modernize-use-noexcept]
+// CHECK-FIXES: void foobar() NOEXCEPT {}
Index: docs/clang-tidy/checks/modernize-use-noexcept.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-noexcept.rst
@@ -0,0 +1,51 @@
+.. title:: clang-tidy - modernize-use-noexcept
+
+modernize-use-noexcept
+==
+
+The check converts exception specifications, e.g., throw(), to either
+noexcept, or a user defined macro.
+
+Example
+---
+
+.. code-block:: c++
+
+  void foo() throw();
+
+transforms to:
+
+.. code-block:: c++
+
+  void foo() noexcept;
+
+
+User defined macros
+---
+
+By default this check will only replace the ``throw()`` and
+``noexcept(true)`` with ``noexcept``.  However, the user can use
+:option:``CandidateStrings" to specify a comma-separated list of macro
+names that will be transformed along with ``throw()`` and
+``noexcept(true)``.
+
+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() NOTHROW {}
+
+transforms to:
+
+.. code-block:: c++
+
+  void foo() NOEXCEPT {}
+
+if the ``CandidateStrings`` option is set to ``NOTHROW``, and the
+``ReplacementString`` option is set to ``NOEXCEPT``.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -83,6 +83,7 @@
modernize-shrink-to-fit
modernize-use-auto
modernize-use-default
+   modernize-use-noexcept
modernize-use-nullptr
modernize-use-override
performance-faster-string-find
Index: clang-tidy/modernize/UseNoexceptCheck.h
===
--- /dev/null
+++ clang-tidy/modernize/UseNoexceptCheck.h
@@ -0,0 +1,49 @@
+//===--- UseNoexceptCheck.h -