stephanemoore created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
stephanemoore updated this revision to Diff 474350.
stephanemoore added a comment.
stephanemoore updated this revision to Diff 474356.
stephanemoore published this revision for review.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
[clang-tidy] Fix namespace comments in AvoidThrowingObjCExceptionCheck.cpp 🧹
This commit fixes namespace comments as suggested by the linter.
stephanemoore added a comment.
Restore original diff that was replaced by accident.
The google-objc-avoid-throwing-exception check enforces the Google
Objective-C Style Guide's prohibition on throwing exceptions in user
code but the check incorrectly triggers findings for code emitted from
system headers. This commit suppresses any findings that do not have
valid locations or are emitted from macros in system headers.
Avoid Throwing Exceptions, Google Objective-C Style Guide:
https://github.com/google/styleguide/blob/gh-pages/objcguide.md#avoid-throwing-exceptions
Test Notes:
Ran clang-tidy lit tests.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D137738
Files:
clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/google/Inputs/system-header-throw.h
clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m
Index:
clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m
===================================================================
---
clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m
+++
clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m
@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy %s google-objc-avoid-throwing-exception %t
+// RUN: %check_clang_tidy %s google-objc-avoid-throwing-exception %t -- -- -I
%S/Inputs/
+
@class NSString;
@interface NSException
@@ -21,12 +22,29 @@
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass in NSError ** instead of
throwing exception to indicate Objective-C errors
[google-objc-avoid-throwing-exception]
}
+#include "system-header-throw.h"
+
+#define THROW(e) @throw e
+
+#define RAISE [NSException raise:@"example" format:@"fmt"]
+
- (void)f2 {
[NSException raise:@"TestException" format:@"Test"];
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: pass in NSError ** instead of
throwing exception to indicate Objective-C errors
[google-objc-avoid-throwing-exception]
[NSException raise:@"TestException" format:@"Test %@" arguments:@"bar"];
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: pass in NSError ** instead of
throwing exception to indicate Objective-C errors
[google-objc-avoid-throwing-exception]
[NotException raise:@"NotException" format:@"Test"];
+
+ NSException *e;
+ SYS_THROW(e);
+
+ SYS_RAISE;
+
+ THROW(e);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass in NSError ** instead of
throwing exception to indicate Objective-C errors
[google-objc-avoid-throwing-exception]
+
+ RAISE;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass in NSError ** instead of
throwing exception to indicate Objective-C errors
[google-objc-avoid-throwing-exception]
}
@end
Index:
clang-tools-extra/test/clang-tidy/checkers/google/Inputs/system-header-throw.h
===================================================================
--- /dev/null
+++
clang-tools-extra/test/clang-tidy/checkers/google/Inputs/system-header-throw.h
@@ -0,0 +1,6 @@
+#pragma clang system_header
+
+#define SYS_THROW(e) @throw e
+
+#define SYS_RAISE [NSException raise:@"example" format:@"fmt"]
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -141,6 +141,10 @@
would be emitted for uninitialized members of an anonymous union despite
there being an initializer for one of the other members.
+- Fixed false positives in :doc:`google-objc-avoid-throwing-exception
+ <clang-tidy/checks/google/objc-avoid-throwing-exception>` check for
exceptions
+ thrown by code emitted from macros in system headers.
+
- Improved :doc:`modernize-use-emplace
<clang-tidy/checks/modernize/use-emplace>`
check.
Index: clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
+++ clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
@@ -36,6 +36,22 @@
Result.Nodes.getNodeAs<ObjCMessageExpr>("raiseException");
auto SourceLoc = MatchedStmt == nullptr ? MatchedExpr->getSelectorStartLoc()
: MatchedStmt->getThrowLoc();
+
+ // Early return on invalid locations.
+ if (SourceLoc.isInvalid())
+ return;
+
+ // If the match location was in a macro, check if the macro was in a system
+ // header.
+ if (SourceLoc.isMacroID()) {
+ SourceManager &SM = *Result.SourceManager;
+ auto MacroLoc = SM.getImmediateMacroCallerLoc(SourceLoc);
+
+ // Matches in system header macros should be ignored.
+ if (SM.isInSystemHeader(MacroLoc))
+ return;
+ }
+
diag(SourceLoc,
"pass in NSError ** instead of throwing exception to indicate "
"Objective-C errors");
Index: clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m
+++ clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m
@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy %s google-objc-avoid-throwing-exception %t
+// RUN: %check_clang_tidy %s google-objc-avoid-throwing-exception %t -- -- -I %S/Inputs/
+
@class NSString;
@interface NSException
@@ -21,12 +22,29 @@
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass in NSError ** instead of throwing exception to indicate Objective-C errors [google-objc-avoid-throwing-exception]
}
+#include "system-header-throw.h"
+
+#define THROW(e) @throw e
+
+#define RAISE [NSException raise:@"example" format:@"fmt"]
+
- (void)f2 {
[NSException raise:@"TestException" format:@"Test"];
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: pass in NSError ** instead of throwing exception to indicate Objective-C errors [google-objc-avoid-throwing-exception]
[NSException raise:@"TestException" format:@"Test %@" arguments:@"bar"];
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: pass in NSError ** instead of throwing exception to indicate Objective-C errors [google-objc-avoid-throwing-exception]
[NotException raise:@"NotException" format:@"Test"];
+
+ NSException *e;
+ SYS_THROW(e);
+
+ SYS_RAISE;
+
+ THROW(e);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass in NSError ** instead of throwing exception to indicate Objective-C errors [google-objc-avoid-throwing-exception]
+
+ RAISE;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass in NSError ** instead of throwing exception to indicate Objective-C errors [google-objc-avoid-throwing-exception]
}
@end
Index: clang-tools-extra/test/clang-tidy/checkers/google/Inputs/system-header-throw.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/google/Inputs/system-header-throw.h
@@ -0,0 +1,6 @@
+#pragma clang system_header
+
+#define SYS_THROW(e) @throw e
+
+#define SYS_RAISE [NSException raise:@"example" format:@"fmt"]
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -141,6 +141,10 @@
would be emitted for uninitialized members of an anonymous union despite
there being an initializer for one of the other members.
+- Fixed false positives in :doc:`google-objc-avoid-throwing-exception
+ <clang-tidy/checks/google/objc-avoid-throwing-exception>` check for exceptions
+ thrown by code emitted from macros in system headers.
+
- Improved :doc:`modernize-use-emplace <clang-tidy/checks/modernize/use-emplace>`
check.
Index: clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
+++ clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
@@ -36,6 +36,22 @@
Result.Nodes.getNodeAs<ObjCMessageExpr>("raiseException");
auto SourceLoc = MatchedStmt == nullptr ? MatchedExpr->getSelectorStartLoc()
: MatchedStmt->getThrowLoc();
+
+ // Early return on invalid locations.
+ if (SourceLoc.isInvalid())
+ return;
+
+ // If the match location was in a macro, check if the macro was in a system
+ // header.
+ if (SourceLoc.isMacroID()) {
+ SourceManager &SM = *Result.SourceManager;
+ auto MacroLoc = SM.getImmediateMacroCallerLoc(SourceLoc);
+
+ // Matches in system header macros should be ignored.
+ if (SM.isInSystemHeader(MacroLoc))
+ return;
+ }
+
diag(SourceLoc,
"pass in NSError ** instead of throwing exception to indicate "
"Objective-C errors");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
