[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
This revision was automatically updated to reflect the committed changes.
Closed by commit rL325572: [clang-tidy] Replace the usage of
std::uncaught_exception with std… (authored by xazax, committed by ).
Herald added subscribers: llvm-commits, klimek.
Changed prior to commit:
https://reviews.llvm.org/D40787?vs=133814=135031#toc
Repository:
rL LLVM
https://reviews.llvm.org/D40787
Files:
clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/UseUncaughtExceptionsCheck.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst
clang-tools-extra/trunk/test/clang-tidy/modernize-use-uncaught-exceptions.cpp
Index: clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -33,6 +33,7 @@
#include "UseNullptrCheck.h"
#include "UseOverrideCheck.h"
#include "UseTransparentFunctorsCheck.h"
+#include "UseUncaughtExceptionsCheck.h"
#include "UseUsingCheck.h"
using namespace clang::ast_matchers;
@@ -78,6 +79,8 @@
CheckFactories.registerCheck("modernize-use-override");
CheckFactories.registerCheck(
"modernize-use-transparent-functors");
+CheckFactories.registerCheck(
+"modernize-use-uncaught-exceptions");
CheckFactories.registerCheck("modernize-use-using");
}
Index: clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
@@ -27,6 +27,7 @@
UseNullptrCheck.cpp
UseOverrideCheck.cpp
UseTransparentFunctorsCheck.cpp
+ UseUncaughtExceptionsCheck.cpp
UseUsingCheck.cpp
LINK_LIBS
Index: clang-tools-extra/trunk/clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp
@@ -0,0 +1,104 @@
+//===--- UseUncaughtExceptionsCheck.cpp - clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "UseUncaughtExceptionsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+void UseUncaughtExceptionsCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus17)
+return;
+
+ std::string MatchText = "::std::uncaught_exception";
+
+ // Using declaration: warning and fix-it.
+ Finder->addMatcher(
+ usingDecl(hasAnyUsingShadowDecl(hasTargetDecl(hasName(MatchText
+ .bind("using_decl"),
+ this);
+
+ // DeclRefExpr: warning, no fix-it.
+ Finder->addMatcher(declRefExpr(allOf(to(functionDecl(hasName(MatchText))),
+ unless(callExpr(
+ .bind("decl_ref_expr"),
+ this);
+
+ // CallExpr: warning, fix-it.
+ Finder->addMatcher(
+ callExpr(allOf(hasDeclaration(functionDecl(hasName(MatchText))),
+ unless(hasAncestor(initListExpr()
+ .bind("call_expr"),
+ this);
+ // CallExpr in initialisation list: warning, fix-it with avoiding narrowing
+ // conversions.
+ Finder->addMatcher(
+ callExpr(allOf(hasAncestor(initListExpr()),
+ hasDeclaration(functionDecl(hasName(MatchText)
+ .bind("init_call_expr"),
+ this);
+}
+
+void UseUncaughtExceptionsCheck::check(const MatchFinder::MatchResult ) {
+ SourceLocation BeginLoc;
+ SourceLocation EndLoc;
+ const CallExpr *C = Result.Nodes.getNodeAs("init_call_expr");
+ bool WarnOnly = false;
+
+ if (C) {
+BeginLoc = C->getLocStart();
+EndLoc = C->getLocEnd();
+ } else if (const auto *E = Result.Nodes.getNodeAs("call_expr")) {
+BeginLoc = E->getLocStart();
+EndLoc = E->getLocEnd();
+ } else if (const auto *D =
+ Result.Nodes.getNodeAs("decl_ref_expr")) {
+BeginLoc = D->getLocStart();
+EndLoc = D->getLocEnd();
+WarnOnly = true;
+ } else {
+const auto *U = Result.Nodes.getNodeAs("using_decl");
+assert(U && "Null
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
koldaniel updated this revision to Diff 133814.
https://reviews.llvm.org/D40787
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp
clang-tidy/modernize/UseUncaughtExceptionsCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst
test/clang-tidy/modernize-use-uncaught-exceptions.cpp
Index: test/clang-tidy/modernize-use-uncaught-exceptions.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-uncaught-exceptions.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy %s modernize-use-uncaught-exceptions %t -- -- -std=c++1z
+#define MACRO std::uncaught_exception
+// CHECK-FIXES: #define MACRO std::uncaught_exception
+
+bool uncaught_exception() {
+ return 0;
+}
+
+namespace std {
+ bool uncaught_exception() {
+return false;
+ }
+
+ int uncaught_exceptions() {
+return 0;
+ }
+}
+
+template
+bool doSomething(T t) {
+ return t();
+ // CHECK-FIXES: return t();
+}
+
+template
+bool doSomething2() {
+ return T();
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: return T();
+}
+
+void no_warn() {
+
+ uncaught_exception();
+ // CHECK-FIXES: uncaught_exception();
+
+ doSomething(uncaught_exception);
+ // CHECK-FIXES: doSomething(uncaught_exception);
+}
+
+void warn() {
+
+ std::uncaught_exception();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: std::uncaught_exceptions();
+
+ using std::uncaught_exception;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: using std::uncaught_exceptions;
+
+ uncaught_exception();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: uncaught_exceptions();
+
+ bool b{uncaught_exception()};
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: bool b{std::uncaught_exceptions() > 0};
+
+ MACRO();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: MACRO();
+
+ doSomething(std::uncaught_exception);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: doSomething(std::uncaught_exception);
+
+ doSomething(uncaught_exception);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: doSomething(uncaught_exception);
+
+ bool (*foo)();
+ foo = _exception;
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: foo = _exception;
+
+ doSomething2();
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: doSomething2();
+}
Index: docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst
@@ -0,0 +1,64 @@
+.. title:: clang-tidy - modernize-use-uncaught-exceptions
+
+modernize-use-uncaught-exceptions
+
+
+This check will warn on calls to ``std::uncaught_exception`` and replace them with
+calls to ``std::uncaught_exceptions``, since ``std::uncaught_exception`` was deprecated
+in C++17.
+
+Below are a few examples of what kind of occurrences will be found and what
+they will be replaced with.
+
+.. code-block:: c++
+
+ #define MACRO1 std::uncaught_exception
+ #define MACRO2 std::uncaught_exception
+
+ int uncaught_exception() {
+ return 0;
+ }
+
+ int main() {
+ int res;
+
+ res = uncaught_exception();
+ // No warning, since it is not the deprecated function from namespace std
+
+ res = MACRO2();
+ // Warning, but will not be replaced
+
+ res = std::uncaught_exception();
+ // Warning and replaced
+
+ using std::uncaught_exception;
+ // Warning and replaced
+
+ res = uncaught_exception();
+ // Warning and replaced
+ }
+
+After applying the fixes the code will look like the following:
+
+.. code-block:: c++
+
+ #define MACRO1 std::uncaught_exception
+ #define MACRO2 std::uncaught_exception
+
+ int uncaught_exception() {
+ return 0;
+ }
+
+ int main() {
+ int res;
+
+ res = uncaught_exception();
+
+ res = MACRO2();
+
+ res = std::uncaught_exceptions();
+
+ using std::uncaught_exceptions;
+
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
This LGTM with a few minor nits to fix.
Comment at: clang-tidy/modernize/ModernizeTidyModule.cpp:80-81
CheckFactories.registerCheck("modernize-use-override");
+CheckFactories.registerCheck(
+"modernize-use-uncaught-exceptions");
CheckFactories.registerCheck(
Please keep this list alphabetized.
Comment at: docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst:19
+int uncaught_exception() {
+return 0;
+}
The indentation of the code examples (here and below) is incorrect.
https://reviews.llvm.org/D40787
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
koldaniel updated this revision to Diff 132595.
https://reviews.llvm.org/D40787
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp
clang-tidy/modernize/UseUncaughtExceptionsCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst
test/clang-tidy/modernize-use-uncaught-exceptions.cpp
Index: test/clang-tidy/modernize-use-uncaught-exceptions.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-uncaught-exceptions.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy %s modernize-use-uncaught-exceptions %t -- -- -std=c++1z
+#define MACRO std::uncaught_exception
+// CHECK-FIXES: #define MACRO std::uncaught_exception
+
+bool uncaught_exception() {
+ return 0;
+}
+
+namespace std {
+ bool uncaught_exception() {
+return false;
+ }
+
+ int uncaught_exceptions() {
+return 0;
+ }
+}
+
+template
+bool doSomething(T t) {
+ return t();
+ // CHECK-FIXES: return t();
+}
+
+template
+bool doSomething2() {
+ return T();
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: return T();
+}
+
+void no_warn() {
+
+ uncaught_exception();
+ // CHECK-FIXES: uncaught_exception();
+
+ doSomething(uncaught_exception);
+ // CHECK-FIXES: doSomething(uncaught_exception);
+}
+
+void warn() {
+
+ std::uncaught_exception();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: std::uncaught_exceptions();
+
+ using std::uncaught_exception;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: using std::uncaught_exceptions;
+
+ uncaught_exception();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: uncaught_exceptions();
+
+ bool b{uncaught_exception()};
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: bool b{std::uncaught_exceptions() > 0};
+
+ MACRO();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: MACRO();
+
+ doSomething(std::uncaught_exception);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: doSomething(std::uncaught_exception);
+
+ doSomething(uncaught_exception);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: doSomething(uncaught_exception);
+
+ bool (*foo)();
+ foo = _exception;
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: foo = _exception;
+
+ doSomething2();
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: doSomething2();
+}
Index: docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst
@@ -0,0 +1,64 @@
+.. title:: clang-tidy - modernize-use-uncaught-exceptions
+
+modernize-use-uncaught-exceptions
+
+
+This check will warn on calls to ``std::uncaught_exception`` and replace them with
+calls to ``std::uncaught_exceptions``, since ``std::uncaught_exception`` was deprecated
+in C++17.
+
+Below are a few examples of what kind of occurrences will be found and what
+they will be replaced with.
+
+.. code-block:: c++
+
+#define MACRO1 std::uncaught_exception
+#define MACRO2 std::uncaught_exception
+
+int uncaught_exception() {
+return 0;
+}
+
+int main() {
+int res;
+
+res = uncaught_exception();
+// No warning, since it is not the deprecated function from namespace std
+
+res = MACRO2();
+// Warning, but will not be replaced
+
+res = std::uncaught_exception();
+// Warning and replaced
+
+using std::uncaught_exception;
+// Warning and replaced
+
+res = uncaught_exception();
+// Warning and replaced
+}
+
+After applying the fixes the code will look like the following:
+
+.. code-block:: c++
+
+#define MACRO1 std::uncaught_exception
+#define MACRO2 std::uncaught_exception
+
+int uncaught_exception() {
+return 0;
+}
+
+int main() {
+int res;
+
+res = uncaught_exception();
+
+res = MACRO2();
+
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
aaron.ballman added inline comments. Comment at: test/clang-tidy/modernize-use-uncaught-exceptions.cpp:64 + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead + // CHECK-FIXES: foo = _exceptions; + koldaniel wrote: > aaron.ballman wrote: > > Applying this fix will break the code so that it no longer compiles. > True, declaration of foo should be changed from type bool to int too. Since > as I can see this could cause a lot of problems, shouldn't be there only a > warning without fixits? I think the right approach here is to warn and not suggest a fix-it. Comment at: test/clang-tidy/modernize-use-uncaught-exceptions.cpp:68 + // CHECK-MESSAGES: [[@LINE-1]]:22: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead + // CHECK-FIXES: res = doSomething2(); + koldaniel wrote: > aaron.ballman wrote: > > This fix seems bad. If the user accepts the fix, then the code will > > diagnose because there's no longer a matching call to `doSomething2()`. > Same type error as earlier, should a fix be applied (changing the type of the > parameter in template definition would be unlucky too, maybe a wrapper > function which could be passed to doSomething2() and does the conversion) or > only a warning? Similarly, I would only warn here as well, and not suggest a fix-it. https://reviews.llvm.org/D40787 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
koldaniel added inline comments. Herald added a subscriber: hintonda. Comment at: test/clang-tidy/modernize-use-uncaught-exceptions.cpp:64 + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead + // CHECK-FIXES: foo = _exceptions; + aaron.ballman wrote: > Applying this fix will break the code so that it no longer compiles. True, declaration of foo should be changed from type bool to int too. Since as I can see this could cause a lot of problems, shouldn't be there only a warning without fixits? Comment at: test/clang-tidy/modernize-use-uncaught-exceptions.cpp:68 + // CHECK-MESSAGES: [[@LINE-1]]:22: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead + // CHECK-FIXES: res = doSomething2(); + aaron.ballman wrote: > This fix seems bad. If the user accepts the fix, then the code will diagnose > because there's no longer a matching call to `doSomething2()`. Same type error as earlier, should a fix be applied (changing the type of the parameter in template definition would be unlucky too, maybe a wrapper function which could be passed to doSomething2() and does the conversion) or only a warning? https://reviews.llvm.org/D40787 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp:45
+
+ if ((C = Result.Nodes.getNodeAs("call_expr"))) {
+BeginLoc = C->getLocStart();
Can remove spurious parens.
Comment at: docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst:7
+This check will warn on calls to ``std::uncaught_exception`` and replace them
with
+calls to ``std::uncaught_exceptions``, since std::uncaught_exception was
deprecated
+in C++17.
Backtick the use of `std::uncaught_exception`.
Comment at: docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst:65
+}
\ No newline at end of file
Please add the newline to the end of the file.
Comment at: test/clang-tidy/modernize-use-uncaught-exceptions.cpp:43
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: 'std::uncaught_exception' is
deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: res = std::uncaught_exceptions() > 0;
+
This is not ideal (the implicit conversion here would do the correct thing).
Comment at: test/clang-tidy/modernize-use-uncaught-exceptions.cpp:45
+
+ using std::uncaught_exception;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: 'std::uncaught_exception' is
deprecated, use 'std::uncaught_exceptions' instead
I'd like to see this, and the other examples that require it, moved into its
own function body (to segregate it from cases we don't want to see the
diagnostics).
Comment at: test/clang-tidy/modernize-use-uncaught-exceptions.cpp:64
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'std::uncaught_exception' is
deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: foo = _exceptions;
+
Applying this fix will break the code so that it no longer compiles.
Comment at: test/clang-tidy/modernize-use-uncaught-exceptions.cpp:68
+ // CHECK-MESSAGES: [[@LINE-1]]:22: warning: 'std::uncaught_exception' is
deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: res = doSomething2();
+
This fix seems bad. If the user accepts the fix, then the code will diagnose
because there's no longer a matching call to `doSomething2()`.
https://reviews.llvm.org/D40787
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
koldaniel updated this revision to Diff 130905.
https://reviews.llvm.org/D40787
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp
clang-tidy/modernize/UseUncaughtExceptionsCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst
test/clang-tidy/modernize-use-uncaught-exceptions.cpp
Index: test/clang-tidy/modernize-use-uncaught-exceptions.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-uncaught-exceptions.cpp
@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy %s modernize-use-uncaught-exceptions %t -- -- -std=c++1z
+#define MACRO std::uncaught_exception
+// CHECK-FIXES: #define MACRO std::uncaught_exception
+
+bool uncaught_exception() {
+ return 0;
+}
+
+namespace std {
+ bool uncaught_exception() {
+return 0;
+ }
+}
+
+template
+bool doSomething(T t) {
+ return t();
+ // CHECK-FIXES: return t();
+}
+
+template
+bool doSomething2() {
+ return T();
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: return T();
+}
+
+int main() {
+ bool res;
+
+ res = uncaught_exception();
+ // CHECK-FIXES: res = uncaught_exception();
+
+ res = doSomething(uncaught_exception);
+ // CHECK-FIXES: res = doSomething(uncaught_exception);
+
+ res = MACRO();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: res = MACRO();
+
+ res = std::uncaught_exception();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: res = std::uncaught_exceptions() > 0;
+
+ using std::uncaught_exception;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: using std::uncaught_exceptions;
+
+ res = uncaught_exception();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: res = std::uncaught_exceptions() > 0;
+
+ res = doSomething(std::uncaught_exception);
+ // CHECK-MESSAGES: [[@LINE-1]]:21: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: res = doSomething(std::uncaught_exceptions);
+
+ res = doSomething(uncaught_exception);
+ // CHECK-MESSAGES: [[@LINE-1]]:21: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: res = doSomething(uncaught_exceptions);
+
+ bool (*foo)();
+ foo = _exception;
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: foo = _exceptions;
+
+ res = doSomething2();
+ // CHECK-MESSAGES: [[@LINE-1]]:22: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: res = doSomething2();
+
+ bool b{std::uncaught_exception()};
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: bool b{std::uncaught_exceptions() > 0};
+
+ return res;
+}
Index: docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst
@@ -0,0 +1,64 @@
+.. title:: clang-tidy - modernize-use-uncaught-exceptions
+
+modernize-use-uncaught-exceptions
+
+
+This check will warn on calls to ``std::uncaught_exception`` and replace them with
+calls to ``std::uncaught_exceptions``, since std::uncaught_exception was deprecated
+in C++17.
+
+Below are a few examples of what kind of occurrences will be found and what
+they will be replaced with.
+
+.. code-block:: c++
+
+#define MACRO1 std::uncaught_exception
+#define MACRO2 std::uncaught_exception
+
+int uncaught_exception() {
+return 0;
+}
+
+int main() {
+int res;
+
+res = uncaught_exception();
+// No warning, since it is not the deprecated function from namespace std
+
+res = MACRO2();
+// Warning, but will not be replaced
+
+res = std::uncaught_exception();
+// Warning and replaced
+
+using std::uncaught_exception;
+// Warning and replaced
+
+res = uncaught_exception();
+// Warning and replaced
+}
+
+After applying the fixes the code will look like the following:
+
+.. code-block:: c++
+
+#define MACRO1 std::uncaught_exception
+#define MACRO2 std::uncaught_exception
+
+int uncaught_exception() {
+return 0;
+}
+
+int main() {
+int res;
+
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
koldaniel added inline comments.
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp:66-67
+}
+Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+ "s");
+ }
aaron.ballman wrote:
> koldaniel wrote:
> > aaron.ballman wrote:
> > > Same concerns here as with the previous review: This fixit can break code
> > > if the code disallows narrowing conversions. e.g.,
> > > ```
> > > bool b{std::uncaught_exception()};
> > > ```
> > > In this case, the fixit will take the deprecated code and make it
> > > ill-formed instead. Perhaps a better fix-it would be to transform the
> > > code into (std::uncaught_exceptions() > 0) to keep the resulting
> > > expression type a bool and still not impact operator precedence.
> > > Alternatively, you can identify the narrowing conversion case and skip
> > > the fix-it entirely in that case (only warn).
> > >
> > > This example should be a test case.
> > If the fix-it would be a transformation to std::uncaught_exceptions() > 0,
> > the code will break in some cases, for example in case of function pointers
> > or template instantiations. Narrowing conversions would not lead to errors,
> > functionality of the code would remain the same.
> > If the fix-it would be a transformation to std::uncaught_exceptions() > 0,
> > the code will break in some cases, for example in case of function pointers
> > or template instantiations.
>
> Very true, this check encompasses more than call expressions, which was
> another concern of mine. For instance, the function pointer case will also
> result in the fix-it breaking code. Further, it may change SFINAE results
> (though changes in SFINAE contexts are less of a concern).
>
> > Narrowing conversions would not lead to errors, functionality of the code
> > would remain the same.
>
> Incorrect; the narrowing conversion will lead to an error depending on the
> context. https://godbolt.org/g/v8tCWM
Fair point, which confused me is that there is no such issue when compiling the
code with gcc instead of clang. In this case, I think the way forward would be
to separate the AST-matchers, and apply a transformation like you proposed when
needed.
https://reviews.llvm.org/D40787
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp:66-67
+}
+Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+ "s");
+ }
koldaniel wrote:
> aaron.ballman wrote:
> > Same concerns here as with the previous review: This fixit can break code
> > if the code disallows narrowing conversions. e.g.,
> > ```
> > bool b{std::uncaught_exception()};
> > ```
> > In this case, the fixit will take the deprecated code and make it
> > ill-formed instead. Perhaps a better fix-it would be to transform the code
> > into (std::uncaught_exceptions() > 0) to keep the resulting expression type
> > a bool and still not impact operator precedence. Alternatively, you can
> > identify the narrowing conversion case and skip the fix-it entirely in that
> > case (only warn).
> >
> > This example should be a test case.
> If the fix-it would be a transformation to std::uncaught_exceptions() > 0,
> the code will break in some cases, for example in case of function pointers
> or template instantiations. Narrowing conversions would not lead to errors,
> functionality of the code would remain the same.
> If the fix-it would be a transformation to std::uncaught_exceptions() > 0,
> the code will break in some cases, for example in case of function pointers
> or template instantiations.
Very true, this check encompasses more than call expressions, which was another
concern of mine. For instance, the function pointer case will also result in
the fix-it breaking code. Further, it may change SFINAE results (though changes
in SFINAE contexts are less of a concern).
> Narrowing conversions would not lead to errors, functionality of the code
> would remain the same.
Incorrect; the narrowing conversion will lead to an error depending on the
context. https://godbolt.org/g/v8tCWM
https://reviews.llvm.org/D40787
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
koldaniel added inline comments.
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp:66-67
+}
+Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+ "s");
+ }
aaron.ballman wrote:
> Same concerns here as with the previous review: This fixit can break code if
> the code disallows narrowing conversions. e.g.,
> ```
> bool b{std::uncaught_exception()};
> ```
> In this case, the fixit will take the deprecated code and make it ill-formed
> instead. Perhaps a better fix-it would be to transform the code into
> (std::uncaught_exceptions() > 0) to keep the resulting expression type a bool
> and still not impact operator precedence. Alternatively, you can identify the
> narrowing conversion case and skip the fix-it entirely in that case (only
> warn).
>
> This example should be a test case.
If the fix-it would be a transformation to std::uncaught_exceptions() > 0, the
code will break in some cases, for example in case of function pointers or
template instantiations. Narrowing conversions would not lead to errors,
functionality of the code would remain the same.
https://reviews.llvm.org/D40787
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp:24
+
+ const char *MatchText = "::std::uncaught_exception";
+
You might as well make this a `std::string` rather than `const char *` because
the `hasName()` matcher accepts that datatype (removes a few implicit
converting constructor calls).
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp:61
+
+// we don't want to modify template definitions
+Text.consume_front("std::");
Comments are full sentences (with correct capitalization and punctuation).
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp:66-67
+}
+Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+ "s");
+ }
Same concerns here as with the previous review: This fixit can break code if
the code disallows narrowing conversions. e.g.,
```
bool b{std::uncaught_exception()};
```
In this case, the fixit will take the deprecated code and make it ill-formed
instead. Perhaps a better fix-it would be to transform the code into
(std::uncaught_exceptions() > 0) to keep the resulting expression type a bool
and still not impact operator precedence. Alternatively, you can identify the
narrowing conversion case and skip the fix-it entirely in that case (only warn).
This example should be a test case.
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.h:19
+
+/// This check will warn for the occurrences of std::uncaught_exception and
replace it with
+/// std::uncaught_exceptions. Since C++17 std::uncaught_exception is
deprecated. In case of
warn for the occurrences of -> warn on calls to
replace it -> replace them with calls to
Comment at: clang-tidy/modernize/UseUncaughtExceptionsCheck.h:20
+/// This check will warn for the occurrences of std::uncaught_exception and
replace it with
+/// std::uncaught_exceptions. Since C++17 std::uncaught_exception is
deprecated. In case of
+/// macro ID there will be only a warning without fixits.
Since C++17... -> std::uncaught_exception was deprecated in C++17.
Comment at: docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst:6-8
+This check will warn for the occurrences of ``std::uncaught_exception`` and
+replace it with ``std::uncaught_exceptions``. Since C++17
+``std::uncaught_exception`` is deprecated.
Same wording suggestions here as above.
https://reviews.llvm.org/D40787
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
koldaniel updated this revision to Diff 128390.
https://reviews.llvm.org/D40787
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp
clang-tidy/modernize/UseUncaughtExceptionsCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst
test/clang-tidy/modernize-use-uncaught-exceptions.cpp
Index: test/clang-tidy/modernize-use-uncaught-exceptions.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-uncaught-exceptions.cpp
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s modernize-use-uncaught-exceptions %t -- -- -std=c++1z
+#define MACRO std::uncaught_exception
+// CHECK-FIXES: #define MACRO std::uncaught_exception
+
+int uncaught_exception() {
+ return 0;
+}
+
+namespace std {
+ int uncaught_exception() {
+return 0;
+ }
+}
+
+template
+int doSomething(T t) {
+ return t();
+ // CHECK-FIXES: return t();
+}
+
+template
+int doSomething2() {
+ return T();
+ // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: return T();
+}
+
+int main() {
+ int res;
+
+ res = uncaught_exception();
+ // CHECK-FIXES: res = uncaught_exception();
+
+ res = doSomething(uncaught_exception);
+ // CHECK-FIXES: res = doSomething(uncaught_exception);
+
+ res = MACRO();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: res = MACRO();
+
+ res = std::uncaught_exception();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: res = std::uncaught_exceptions();
+
+ using std::uncaught_exception;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: using std::uncaught_exceptions;
+
+ res = uncaught_exception();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: res = uncaught_exceptions();
+
+ res = doSomething(std::uncaught_exception);
+ // CHECK-MESSAGES: [[@LINE-1]]:21: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: res = doSomething(std::uncaught_exceptions);
+
+ res = doSomething(uncaught_exception);
+ // CHECK-MESSAGES: [[@LINE-1]]:21: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: res = doSomething(uncaught_exceptions);
+
+ int (*foo)();
+ foo = _exception;
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: foo = _exceptions;
+
+ res = doSomething2();
+ // CHECK-MESSAGES: [[@LINE-1]]:22: warning: 'std::uncaught_exception' is deprecated, use 'std::uncaught_exceptions' instead
+ // CHECK-FIXES: res = doSomething2();
+
+ return res;
+}
Index: docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-uncaught-exceptions.rst
@@ -0,0 +1,64 @@
+.. title:: clang-tidy - modernize-use-uncaught-exceptions
+
+modernize-use-uncaught-exceptions
+
+
+This check will warn for the occurrences of ``std::uncaught_exception`` and
+replace it with ``std::uncaught_exceptions``. Since C++17
+``std::uncaught_exception`` is deprecated.
+
+Below are a few examples of what kind of occurrences will be found and what
+they will be replaced with.
+
+.. code-block:: c++
+
+#define MACRO1 std::uncaught_exception
+#define MACRO2 std::uncaught_exception
+
+int uncaught_exception() {
+return 0;
+}
+
+int main() {
+int res;
+
+res = uncaught_exception();
+// No warning, since it is not the deprecated function from namespace std
+
+res = MACRO2();
+// Warning, but will not be replaced
+
+res = std::uncaught_exception();
+// Warning and replaced
+
+using std::uncaught_exception;
+// Warning and replaced
+
+res = uncaught_exception();
+// Warning and replaced
+}
+
+After applying the fixes the code will look like the following:
+
+.. code-block:: c++
+
+#define MACRO1 std::uncaught_exception
+#define MACRO2 std::uncaught_exception
+
+int uncaught_exception() {
+return 0;
+}
+
+int main() {
+int res;
+
+res = uncaught_exception();
+
+res = MACRO2();
+
+res = std::uncaught_exceptions();
+
+using std::uncaught_exceptions;
+
+res = uncaught_exceptions();
+}
\ No newline at end of file
Index:
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:59
+ if (!BeginLoc.isMacroID()) {
+Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+ "s");
xazax.hun wrote:
> aaron.ballman wrote:
> > JonasToth wrote:
> > > koldaniel wrote:
> > > > JonasToth wrote:
> > > > > Could the location not simply be `EndLoc`?
> > > > As I could see, when EndLoc was used in Diag (diag(..) of
> > > > CreateInsertion(...) methods, it still pointed to the beginning of the
> > > > token marking the whole call. So if the CreateInsertion function looked
> > > > like this:
> > > >
> > > > Diag <<
> > > > FixItHint::CreateInsertion(EndLoc.getLocWithOffset(TextLength), "s");
> > > >
> > > > had the same effect after applying the fix its as
> > > >
> > > > Diag <<
> > > > FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength), "s");
> > > >
> > > > for calls like 'uncaught_exception()' (without std:: namespace written
> > > > at the beginning, because it increases TextLength, and in case of
> > > > EndLoc the offset will be counted from the beginning of the function
> > > > name, not the namespace identifier).
> > > Thats odd. You could do a Replacement with `getSourceRange` probably.
> > > Sounds more inefficient, but might be shorter in Code.
> > This fixit can break code if the code disallows narrowing conversions. e.g.,
> > ```
> > bool b{std::uncaught_exception()};
> > ```
> > In this case, the fixit will take the deprecated code and make it
> > ill-formed instead. Perhaps a better fix-it would be to transform the code
> > into `(std::uncaught_exceptions() > 0)` to keep the resulting expression
> > type a `bool` and still not impact operator precedence?
> Good point, but this kind of fix it is a bit ugly. Maybe skipping the fixit
> in narrowing cases or only generate the more complicated replacement in the
> narrowing case would be nicer.
I would be fine with only using the uglier version if there's a narrowing
conversion, or removing the fixit entirely in the narrowing case.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40787
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
xazax.hun added inline comments.
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:59
+ if (!BeginLoc.isMacroID()) {
+Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+ "s");
aaron.ballman wrote:
> JonasToth wrote:
> > koldaniel wrote:
> > > JonasToth wrote:
> > > > Could the location not simply be `EndLoc`?
> > > As I could see, when EndLoc was used in Diag (diag(..) of
> > > CreateInsertion(...) methods, it still pointed to the beginning of the
> > > token marking the whole call. So if the CreateInsertion function looked
> > > like this:
> > >
> > > Diag << FixItHint::CreateInsertion(EndLoc.getLocWithOffset(TextLength),
> > > "s");
> > >
> > > had the same effect after applying the fix its as
> > >
> > > Diag <<
> > > FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength), "s");
> > >
> > > for calls like 'uncaught_exception()' (without std:: namespace written at
> > > the beginning, because it increases TextLength, and in case of EndLoc the
> > > offset will be counted from the beginning of the function name, not the
> > > namespace identifier).
> > Thats odd. You could do a Replacement with `getSourceRange` probably.
> > Sounds more inefficient, but might be shorter in Code.
> This fixit can break code if the code disallows narrowing conversions. e.g.,
> ```
> bool b{std::uncaught_exception()};
> ```
> In this case, the fixit will take the deprecated code and make it ill-formed
> instead. Perhaps a better fix-it would be to transform the code into
> `(std::uncaught_exceptions() > 0)` to keep the resulting expression type a
> `bool` and still not impact operator precedence?
Good point, but this kind of fix it is a bit ugly. Maybe skipping the fixit in
narrowing cases or only generate the more complicated replacement in the
narrowing case would be nicer.
Comment at: test/clang-tidy/modernize-replace-uncaught-exception.cpp:16
+template
+int doSomething(T t) {
+return t();
It would be great to have a test where the template parameter is a function
pointer and it is called with `uncaught_exception`. And with a check fixes make
sure that the definition of the template is untouched.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40787
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:59
+ if (!BeginLoc.isMacroID()) {
+Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+ "s");
JonasToth wrote:
> koldaniel wrote:
> > JonasToth wrote:
> > > Could the location not simply be `EndLoc`?
> > As I could see, when EndLoc was used in Diag (diag(..) of
> > CreateInsertion(...) methods, it still pointed to the beginning of the
> > token marking the whole call. So if the CreateInsertion function looked
> > like this:
> >
> > Diag << FixItHint::CreateInsertion(EndLoc.getLocWithOffset(TextLength),
> > "s");
> >
> > had the same effect after applying the fix its as
> >
> > Diag <<
> > FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength), "s");
> >
> > for calls like 'uncaught_exception()' (without std:: namespace written at
> > the beginning, because it increases TextLength, and in case of EndLoc the
> > offset will be counted from the beginning of the function name, not the
> > namespace identifier).
> Thats odd. You could do a Replacement with `getSourceRange` probably. Sounds
> more inefficient, but might be shorter in Code.
This fixit can break code if the code disallows narrowing conversions. e.g.,
```
bool b{std::uncaught_exception()};
```
In this case, the fixit will take the deprecated code and make it ill-formed
instead. Perhaps a better fix-it would be to transform the code into
`(std::uncaught_exceptions() > 0)` to keep the resulting expression type a
`bool` and still not impact operator precedence?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40787
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
JonasToth added inline comments.
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:32
+ Finder->addMatcher(
+ declRefExpr(to(functionDecl(hasName(MatchText.bind("call_expr"),
+ this);
koldaniel wrote:
> JonasToth wrote:
> > Interesting. Did you consider `callExpr` as well? I never used
> > `declRefExpr` in this context but it might be a good choice too. Would it
> > catch taking a function pointer to `std::uncaught_exception` too? If so, i
> > need to overthink some of my code :)
> According to the original concept callExpr was used, but to match template
> instantiations and other usages than calling the function directly, it seemed
> to me a better and simpler solution to use declRefExpr. In case of function
> pointer initializations like the this:
>
> bool (*foo)();
> foo = ::uncaught_exception;
> res = foo();
>
> there will be a warning and the fix will be applied too.
Could you add testcases for function pointers please?
Having tests for the template instantiations (in which form? using
`std::uncaught_exceptions` as template argument for callables?) would be a good
thing as well.
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:59
+ if (!BeginLoc.isMacroID()) {
+Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+ "s");
koldaniel wrote:
> JonasToth wrote:
> > Could the location not simply be `EndLoc`?
> As I could see, when EndLoc was used in Diag (diag(..) of
> CreateInsertion(...) methods, it still pointed to the beginning of the token
> marking the whole call. So if the CreateInsertion function looked like this:
>
> Diag << FixItHint::CreateInsertion(EndLoc.getLocWithOffset(TextLength),
> "s");
>
> had the same effect after applying the fix its as
>
> Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
> "s");
>
> for calls like 'uncaught_exception()' (without std:: namespace written at the
> beginning, because it increases TextLength, and in case of EndLoc the offset
> will be counted from the beginning of the function name, not the namespace
> identifier).
Thats odd. You could do a Replacement with `getSourceRange` probably. Sounds
more inefficient, but might be shorter in Code.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40787
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
Eugene.Zelenko added a comment. I think will be good idea to rename check to //modernize-use-uncaught-exceptions// to be consistent with other similar checks. Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40787 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
koldaniel added inline comments.
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:32
+ Finder->addMatcher(
+ declRefExpr(to(functionDecl(hasName(MatchText.bind("call_expr"),
+ this);
JonasToth wrote:
> Interesting. Did you consider `callExpr` as well? I never used `declRefExpr`
> in this context but it might be a good choice too. Would it catch taking a
> function pointer to `std::uncaught_exception` too? If so, i need to overthink
> some of my code :)
According to the original concept callExpr was used, but to match template
instantiations and other usages than calling the function directly, it seemed
to me a better and simpler solution to use declRefExpr. In case of function
pointer initializations like the this:
bool (*foo)();
foo = ::uncaught_exception;
res = foo();
there will be a warning and the fix will be applied too.
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:59
+ if (!BeginLoc.isMacroID()) {
+Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+ "s");
JonasToth wrote:
> Could the location not simply be `EndLoc`?
As I could see, when EndLoc was used in Diag (diag(..) of CreateInsertion(...)
methods, it still pointed to the beginning of the token marking the whole
call. So if the CreateInsertion function looked like this:
Diag << FixItHint::CreateInsertion(EndLoc.getLocWithOffset(TextLength), "s");
had the same effect after applying the fix its as
Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
"s");
for calls like 'uncaught_exception()' (without std:: namespace written at the
beginning, because it increases TextLength, and in case of EndLoc the offset
will be counted from the beginning of the function name, not the namespace
identifier).
Comment at: test/clang-tidy/modernize-replace-uncaught-exception.cpp:41
+
+ res = uncaught_exception();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: 'std::uncaught_exception' is
deprecated, use 'std::uncaught_exceptions' instead
xazax.hun wrote:
> Why is this call not ambiguous (global namespace's functions vs std's)?
Global namespace's function will be hidden because of the using declaration, it
can be called via ::uncaught_exception(). The call would be ambiguous in case
of using namespace:
bool uncaught_exception()
{
return true;
}
int main()
{
std::cout<<"std: "<
[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions
JonasToth added inline comments.
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:20
+
+static const char *MatchText = "::std::uncaught_exception";
+
Could that be a local variable in `registerMatchers`? Even though its static
and const it might be best to reduce its scope as much as possible.
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:32
+ Finder->addMatcher(
+ declRefExpr(to(functionDecl(hasName(MatchText.bind("call_expr"),
+ this);
Interesting. Did you consider `callExpr` as well? I never used `declRefExpr` in
this context but it might be a good choice too. Would it catch taking a
function pointer to `std::uncaught_exception` too? If so, i need to overthink
some of my code :)
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:45
+ } else {
+const auto *U = Result.Nodes.getNodeAs("using_decl");
+BeginLoc = U->getNameInfo().getBeginLoc();
The implicit assumption here is that one of both must have been matched which
is true now.
But adding an `assert` on `U` might still be a good thing. You never known how
the code evolves and what bugs might come alive.
That just ensures that there is no possible way to dereference a `nullptr`.
Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:59
+ if (!BeginLoc.isMacroID()) {
+Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+ "s");
Could the location not simply be `EndLoc`?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40787
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
