From 6c2140943cbe257c85f7121349c5bca950a26e0d Mon Sep 17 00:00:00 2001 From: Stephane Moore Date: Wed, 9 Nov 2022 11:07:53 -0800 Subject: [PATCH] =?utf8?q?[clang-tidy]=20Suppress=20google-objc-avoid-thro?= =?utf8?q?wing-exception=20in=20system=20macros=20=F0=9F=AB=A2?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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. Reviewed By: gribozavr2 Differential Revision: https://reviews.llvm.org/D137738 --- .../google/AvoidThrowingObjCExceptionCheck.cpp | 16 ++++++++++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++ .../checkers/google/Inputs/system-header-throw.h | 6 ++++++ .../checkers/google/objc-avoid-throwing-exception.m | 20 +++++++++++++++++++- 4 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/google/Inputs/system-header-throw.h diff --git a/clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp b/clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp index 2263806..9205125 100644 --- a/clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp +++ b/clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp @@ -36,6 +36,22 @@ void AvoidThrowingObjCExceptionCheck::check( Result.Nodes.getNodeAs("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"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 44c3743..52346e5 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -141,6 +141,10 @@ Changes in existing checks 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 + ` check for exceptions + thrown by code emitted from macros in system headers. + - Improved :doc:`modernize-use-emplace ` check. diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/system-header-throw.h b/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/system-header-throw.h new file mode 100644 index 0000000..22d1bd4 --- /dev/null +++ b/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"] + diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m b/clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m index 7fa32e7..c28bf64 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m +++ b/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 -- 2.7.4