[PATCH] D137738: [clang-tidy] Suppress google-objc-avoid-throwing-exception in system macros 🫢

[Stephane Moore via Phabricator via cfe-commits]

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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D137738.474356.patch
Type: text/x-patch
Size: 4027 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221109/f0782813/attachment-0001.bin>