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>

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@158130 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Lex/Lexer.h b/include/clang/Lex/Lexer.h
index e96e1d7cb792d80cd9775caad8837fd6d48f32d9..4fbc858fef033414f3afc5e60488c424d7887aca 100644 (file)
--- a/include/clang/Lex/Lexer.h
+++ b/include/clang/Lex/Lexer.h
@@ -541,6 +541,9 @@ public:
                                          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

diff --git a/lib/ARCMigrate/TransUnbridgedCasts.cpp b/lib/ARCMigrate/TransUnbridgedCasts.cpp
index 72c0d8e7de38b041dd01989f5f6ae639f6863966..f8bc5cf932c0e2af81dc6739cd3f85205f80c9a6 100644 (file)
--- a/lib/ARCMigrate/TransUnbridgedCasts.cpp
+++ b/lib/ARCMigrate/TransUnbridgedCasts.cpp
@@ -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 @@ private:
       }
     } 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(), ")");
       }
     }

diff --git a/lib/Lex/Lexer.cpp b/lib/Lex/Lexer.cpp
index 4ab0dae1fa16315e2e935a2da24f52d01afc0e6b..a806ce34e236c0a83613b2ac7266a3de85bc10f7 100644 (file)
--- a/lib/Lex/Lexer.cpp
+++ b/lib/Lex/Lexer.cpp
@@ -1124,6 +1124,11 @@ static inline bool isRawStringDelimBody(unsigned char c) {
     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.

diff --git a/lib/Sema/SemaExprObjC.cpp b/lib/Sema/SemaExprObjC.cpp
index 1b0ba58084129d2ea3b3668e34446434f0190370..ed1a112a7eb75050016a281bb90136e6f7b4c363 100644 (file)
--- a/lib/Sema/SemaExprObjC.cpp
+++ b/lib/Sema/SemaExprObjC.cpp
@@ -2820,14 +2820,23 @@ static void addFixitForObjCARCConversion(Sema &S,
       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()),
                                        ")"));

diff --git a/test/SemaObjC/arc-bridged-cast.m b/test/SemaObjC/arc-bridged-cast.m
index 56efe81d608b0d1a84f96157d456343d4c7e2487..c8f4d0d1661af0bc2062bd1ceb1119363cf90ae3 100644 (file)
--- a/test/SemaObjC/arc-bridged-cast.m
+++ b/test/SemaObjC/arc-bridged-cast.m
@@ -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 @@ void to_cf(id obj) {
 
   // 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"
 }