From 07fe64cddfe450fb0b1716958a04758f4b2ceae7 Mon Sep 17 00:00:00 2001 From: George Karpenkov Date: Mon, 14 May 2018 20:29:16 +0000 Subject: [PATCH] Remove the fixit for the diagnostics regarding capturing autoreleasing variables in a block The fixit is actively harmful, as it encourages developers to ignore the warning and to write unsafe code. It is almost impossible to write safe code while capturing autoreleasing variables in the block, as in order to check that the block is never called in the autoreleasing pool the developer has to check the transitive closure of all potential callers of the block. Differential Revision: https://reviews.llvm.org/D46778 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@332288 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 2 -- lib/Sema/SemaExpr.cpp | 24 ---------------------- test/SemaObjC/arc.m | 20 +++++++----------- 3 files changed, 7 insertions(+), 39 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 2b0c2a8a7b..3aee192216 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -5435,8 +5435,6 @@ def warn_block_capture_autoreleasing : Warning< "block captures an autoreleasing out-parameter, which may result in " "use-after-free bugs">, InGroup; -def note_declare_parameter_autoreleasing : Note< - "declare the parameter __autoreleasing explicitly to suppress this warning">; def note_declare_parameter_strong : Note< "declare the parameter __strong or capture a __block __strong variable to " "keep values alive across autorelease pools">; diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 8d0de0b66c..aeedd6b169 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -14579,30 +14579,6 @@ static bool captureInBlock(BlockScopeInfo *BSI, VarDecl *Var, if (BuildAndDiagnose) { SourceLocation VarLoc = Var->getLocation(); S.Diag(Loc, diag::warn_block_capture_autoreleasing); - { - auto AddAutoreleaseNote = - S.Diag(VarLoc, diag::note_declare_parameter_autoreleasing); - // Provide a fix-it for the '__autoreleasing' keyword at the - // appropriate location in the variable's type. - if (const auto *TSI = Var->getTypeSourceInfo()) { - PointerTypeLoc PTL = - TSI->getTypeLoc().getAsAdjusted(); - if (PTL) { - SourceLocation Loc = PTL.getPointeeLoc().getEndLoc(); - Loc = Lexer::getLocForEndOfToken(Loc, 0, S.getSourceManager(), - S.getLangOpts()); - if (Loc.isValid()) { - StringRef CharAtLoc = Lexer::getSourceText( - CharSourceRange::getCharRange(Loc, Loc.getLocWithOffset(1)), - S.getSourceManager(), S.getLangOpts()); - AddAutoreleaseNote << FixItHint::CreateInsertion( - Loc, CharAtLoc.empty() || !isWhitespace(CharAtLoc[0]) - ? " __autoreleasing " - : " __autoreleasing"); - } - } - } - } S.Diag(VarLoc, diag::note_declare_parameter_strong); } } diff --git a/test/SemaObjC/arc.m b/test/SemaObjC/arc.m index e90d0a87e3..fcac798ff2 100644 --- a/test/SemaObjC/arc.m +++ b/test/SemaObjC/arc.m @@ -1,5 +1,5 @@ // RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -fblocks -verify -Wno-objc-root-class %s -// RUN: not %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -fblocks -Wno-objc-root-class -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: not %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -fblocks -Wno-objc-root-class -fdiagnostics-parseable-fixits %s 2>&1 typedef unsigned long NSUInteger; typedef const void * CFTypeRef; @@ -811,23 +811,17 @@ int garf() { } void block_capture_autoreleasing(A * __autoreleasing *a, - A **b, // expected-note {{declare the parameter __autoreleasing explicitly to suppress this warning}} expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}} - A * _Nullable *c, // expected-note {{declare the parameter __autoreleasing explicitly to suppress this warning}} expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}} + A **b, // expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}} + A * _Nullable *c, // expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}} A * _Nullable __autoreleasing *d, - A ** _Nullable e, // expected-note {{declare the parameter __autoreleasing explicitly to suppress this warning}} expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}} + A ** _Nullable e, // expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}} A * __autoreleasing * _Nullable f, id __autoreleasing *g, - id *h, // expected-note {{declare the parameter __autoreleasing explicitly to suppress this warning}} expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}} - id _Nullable *i, // expected-note {{declare the parameter __autoreleasing explicitly to suppress this warning}} expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}} + id *h, // expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}} + id _Nullable *i, // expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}} id _Nullable __autoreleasing *j, - id * _Nullable k, // expected-note {{declare the parameter __autoreleasing explicitly to suppress this warning}} expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}} + id * _Nullable k, // expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}} id __autoreleasing * _Nullable l) { - // CHECK: fix-it:"{{.*}}":{[[@LINE-11]]:37-[[@LINE-11]]:37}:" __autoreleasing " - // CHECK: fix-it:"{{.*}}":{[[@LINE-11]]:47-[[@LINE-11]]:47}:" __autoreleasing" - // CHECK: fix-it:"{{.*}}":{[[@LINE-10]]:37-[[@LINE-10]]:37}:" __autoreleasing " - // CHECK: fix-it:"{{.*}}":{[[@LINE-8]]:36-[[@LINE-8]]:36}:" __autoreleasing" - // CHECK: fix-it:"{{.*}}":{[[@LINE-8]]:46-[[@LINE-8]]:46}:" __autoreleasing" - // CHECK: fix-it:"{{.*}}":{[[@LINE-7]]:36-[[@LINE-7]]:36}:" __autoreleasing" ^{ (void)*a; (void)*b; // expected-warning {{block captures an autoreleasing out-parameter, which may result in use-after-free bugs}} -- 2.40.0