[cfe-commits] r158130 - in /cfe/trunk: include/clang/Lex/Lexer.h lib/ARCMigrate/TransUnbridgedCasts.cpp lib/Lex/Lexer.cpp lib/Sema/SemaExprObjC.cpp test/SemaObjC/arc-bridged-cast.m
Argyrios Kyrtzidis
kyrtzidis at apple.com
Thu Jun 7 11:07:20 PDT 2012
On Jun 6, 2012, at 6:10 PM, Jordan Rose wrote:
> Author: jrose
> Date: Wed Jun 6 20:10:31 2012
> New Revision: 158130
>
> URL: http://llvm.org/viewvc/llvm-project?rev=158130&view=rev
> Log:
> Insert a space if necessary when suggesting CFBridgingRetain/Release.
>
> This was a problem for people who write 'return(result);'
>
> Also fix ARCMT's corresponding code, though there's no test case for this
> because implicit casts like this are rejected by the migrator for being
> ambiguous, and explicit casts have no problem.
>
> <rdar://problem/11577346>
Thanks for taking care of ARCMT as well, in case it will be necessary in the future.
In general, making sure that a space is inserted when necessary, seems like it should be handled by the fixit mechanism and it shouldn't be something that one has to worry about when creating the note+fixit.
Fixits go through the "Commit" mechanism (see mergeFixits in DiagnosticRenderer.cpp) to make sure they are composed correctly and macro expansions are handled. Seems like that mechanism should also take care of that, I'll take a look into it at some point.
-Argyrios
>
> Modified:
> cfe/trunk/include/clang/Lex/Lexer.h
> cfe/trunk/lib/ARCMigrate/TransUnbridgedCasts.cpp
> cfe/trunk/lib/Lex/Lexer.cpp
> cfe/trunk/lib/Sema/SemaExprObjC.cpp
> cfe/trunk/test/SemaObjC/arc-bridged-cast.m
>
> Modified: cfe/trunk/include/clang/Lex/Lexer.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Lexer.h?rev=158130&r1=158129&r2=158130&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Lex/Lexer.h (original)
> +++ cfe/trunk/include/clang/Lex/Lexer.h Wed Jun 6 20:10:31 2012
> @@ -541,6 +541,9 @@
> const LangOptions &LangOpts,
> bool SkipTrailingWhitespaceAndNewLine);
>
> + /// \brief Returns true if the given character could appear in an identifier.
> + static bool isIdentifierBodyChar(char c, const LangOptions &LangOpts);
> +
> private:
>
> /// getCharAndSizeSlowNoWarn - Same as getCharAndSizeSlow, but never emits a
>
> Modified: cfe/trunk/lib/ARCMigrate/TransUnbridgedCasts.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/TransUnbridgedCasts.cpp?rev=158130&r1=158129&r2=158130&view=diff
> ==============================================================================
> --- cfe/trunk/lib/ARCMigrate/TransUnbridgedCasts.cpp (original)
> +++ cfe/trunk/lib/ARCMigrate/TransUnbridgedCasts.cpp Wed Jun 6 20:10:31 2012
> @@ -37,6 +37,7 @@
> #include "clang/Analysis/DomainSpecific/CocoaConventions.h"
> #include "clang/Sema/SemaDiagnostic.h"
> #include "clang/AST/ParentMap.h"
> +#include "clang/Lex/Lexer.h"
> #include "clang/Basic/SourceManager.h"
> #include "llvm/ADT/SmallString.h"
>
> @@ -229,20 +230,26 @@
> }
> } else {
> assert(Kind == OBC_BridgeTransfer || Kind == OBC_BridgeRetained);
> - StringRef cfBridging;
> + SmallString<32> BridgeCall;
> +
> + Expr *WrapE = E->getSubExpr();
> + SourceLocation InsertLoc = WrapE->getLocStart();
> +
> + SourceManager &SM = Pass.Ctx.getSourceManager();
> + char PrevChar = *SM.getCharacterData(InsertLoc.getLocWithOffset(-1));
> + if (Lexer::isIdentifierBodyChar(PrevChar, Pass.Ctx.getLangOpts()))
> + BridgeCall += ' ';
> +
> if (Kind == OBC_BridgeTransfer)
> - cfBridging = "CFBridgingRelease";
> + BridgeCall += "CFBridgingRelease";
> else
> - cfBridging = "CFBridgingRetain";
> + BridgeCall += "CFBridgingRetain";
>
> - Expr *WrapE = E->getSubExpr();
> - SourceLocation insertLoc = WrapE->getLocStart();
> if (isa<ParenExpr>(WrapE)) {
> - TA.insert(insertLoc, cfBridging);
> + TA.insert(InsertLoc, BridgeCall);
> } else {
> - std::string withParens = cfBridging;
> - withParens += '(';
> - TA.insert(insertLoc, withParens);
> + BridgeCall += '(';
> + TA.insert(InsertLoc, BridgeCall);
> TA.insertAfterToken(WrapE->getLocEnd(), ")");
> }
> }
>
> Modified: cfe/trunk/lib/Lex/Lexer.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Lexer.cpp?rev=158130&r1=158129&r2=158130&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/Lexer.cpp (original)
> +++ cfe/trunk/lib/Lex/Lexer.cpp Wed Jun 6 20:10:31 2012
> @@ -1124,6 +1124,11 @@
> true : false;
> }
>
> +// Allow external clients to make use of CharInfo.
> +bool Lexer::isIdentifierBodyChar(char c, const LangOptions &LangOpts) {
> + return isIdentifierBody(c) || (c == '$' && LangOpts.DollarIdents);
> +}
> +
>
> //===----------------------------------------------------------------------===//
> // Diagnostics forwarding code.
>
> Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=158130&r1=158129&r2=158130&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Wed Jun 6 20:10:31 2012
> @@ -2820,14 +2820,23 @@
> castedE = CCE->getSubExpr();
> castedE = castedE->IgnoreImpCasts();
> SourceRange range = castedE->getSourceRange();
> +
> + SmallString<32> BridgeCall;
> +
> + SourceManager &SM = S.getSourceManager();
> + char PrevChar = *SM.getCharacterData(range.getBegin().getLocWithOffset(-1));
> + if (Lexer::isIdentifierBodyChar(PrevChar, S.getLangOpts()))
> + BridgeCall += ' ';
> +
> + BridgeCall += CFBridgeName;
> +
> if (isa<ParenExpr>(castedE)) {
> DiagB.AddFixItHint(FixItHint::CreateInsertion(range.getBegin(),
> - CFBridgeName));
> + BridgeCall));
> } else {
> - std::string namePlusParen = CFBridgeName;
> - namePlusParen += "(";
> + BridgeCall += '(';
> DiagB.AddFixItHint(FixItHint::CreateInsertion(range.getBegin(),
> - namePlusParen));
> + BridgeCall));
> DiagB.AddFixItHint(FixItHint::CreateInsertion(
> S.PP.getLocForEndOfToken(range.getEnd()),
> ")"));
>
> Modified: cfe/trunk/test/SemaObjC/arc-bridged-cast.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-bridged-cast.m?rev=158130&r1=158129&r2=158130&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/arc-bridged-cast.m (original)
> +++ cfe/trunk/test/SemaObjC/arc-bridged-cast.m Wed Jun 6 20:10:31 2012
> @@ -1,4 +1,5 @@
> // RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fsyntax-only -fobjc-arc -fblocks -verify %s
> +// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fsyntax-only -fobjc-arc -fblocks -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
>
> typedef const void *CFTypeRef;
> CFTypeRef CFBridgingRetain(id X);
> @@ -32,13 +33,35 @@
>
> // rdar://problem/9629566 - temporary workaround
> CFTypeRef cf5 = (__bridge_retain CFTypeRef)CreateSomething(); // expected-error {{unknown cast annotation __bridge_retain; did you mean __bridge_retained?}}
> + // CHECK: fix-it:"{{.*}}":{35:20-35:35}:"__bridge_retained"
> }
>
> -void fixits() {
> +CFTypeRef fixits() {
> id obj1 = (id)CFCreateSomething(); // expected-error{{cast of C pointer type 'CFTypeRef' (aka 'const void *') to Objective-C pointer type 'id' requires a bridged cast}} \
> // expected-note{{use __bridge to convert directly (no change in ownership)}} \
> // expected-note{{use CFBridgingRelease call to transfer ownership of a +1 'CFTypeRef' (aka 'const void *') into ARC}}
> + // CHECK: fix-it:"{{.*}}":{40:14-40:14}:"__bridge "
> + // CHECK: fix-it:"{{.*}}":{40:17-40:17}:"CFBridgingRelease("
> + // CHECK: fix-it:"{{.*}}":{40:36-40:36}:")"
> +
> CFTypeRef cf1 = (CFTypeRef)CreateSomething(); // expected-error{{cast of Objective-C pointer type 'id' to C pointer type 'CFTypeRef' (aka 'const void *') requires a bridged cast}} \
> // expected-note{{use __bridge to convert directly (no change in ownership)}} \
> // expected-note{{use CFBridgingRetain call to make an ARC object available as a +1 'CFTypeRef' (aka 'const void *')}}
> + // CHECK: fix-it:"{{.*}}":{47:20-47:20}:"__bridge "
> + // CHECK: fix-it:"{{.*}}":{47:30-47:30}:"CFBridgingRetain("
> + // CHECK: fix-it:"{{.*}}":{47:47-47:47}:")"
> +
> + return (obj1); // expected-error{{implicit conversion of Objective-C pointer type 'id' to C pointer type 'CFTypeRef' (aka 'const void *') requires a bridged cast}} \
> + // expected-note{{use __bridge to convert directly (no change in ownership)}} \
> + // expected-note{{use CFBridgingRetain call to make an ARC object available as a +1 'CFTypeRef' (aka 'const void *')}}
> + // CHECK: fix-it:"{{.*}}":{54:10-54:10}:"(__bridge CFTypeRef)"
> + // CHECK: fix-it:"{{.*}}":{54:10-54:10}:"CFBridgingRetain"
> +}
> +
> +CFTypeRef fixitsWithSpace(id obj) {
> + return(obj); // expected-error{{implicit conversion of Objective-C pointer type 'id' to C pointer type 'CFTypeRef' (aka 'const void *') requires a bridged cast}} \
> + // expected-note{{use __bridge to convert directly (no change in ownership)}} \
> + // expected-note{{use CFBridgingRetain call to make an ARC object available as a +1 'CFTypeRef' (aka 'const void *')}}
> + // CHECK: fix-it:"{{.*}}":{62:9-62:9}:"(__bridge CFTypeRef)"
> + // CHECK: fix-it:"{{.*}}":{62:9-62:9}:" CFBridgingRetain"
> }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list