Selector::getIdentifierInfoForSlot() can return NULL values, a fact

that was ignored in a few places (most notably, code
completion). Introduce Selector::getNameForSlot() for the common case
where we only care about the name. Audit all uses of
getIdentifierInfoForSlot(), switching many over to getNameForSlot(),
fixing a few crashers.

Fixed <rdar://problem/8939352>, a code-completion crasher.

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

diff --git a/include/clang/Basic/IdentifierTable.h b/include/clang/Basic/IdentifierTable.h
index 065ea0fee55b03ec53d83d89b568823988406670..d576643550bd15c48209b449eda0c87ea60847be 100644 (file)
--- a/include/clang/Basic/IdentifierTable.h
+++ b/include/clang/Basic/IdentifierTable.h
@@ -510,8 +510,33 @@ public:
     return getIdentifierInfoFlag() == ZeroArg;
   }
   unsigned getNumArgs() const;
+  
+  
+  /// \brief Retrieve the identifier at a given position in the selector.
+  ///
+  /// Note that the identifier pointer returned may be NULL. Clients that only
+  /// care about the text of the identifier string, and not the specific, 
+  /// uniqued identifier pointer, should use \c getNameForSlot(), which returns
+  /// an empty string when the identifier pointer would be NULL.
+  ///
+  /// \param argIndex The index for which we want to retrieve the identifier.
+  /// This index shall be less than \c getNumArgs() unless this is a keyword
+  /// selector, in which case 0 is the only permissible value.
+  ///
+  /// \returns the uniqued identifier for this slot, or NULL if this slot has
+  /// no corresponding identifier.
   IdentifierInfo *getIdentifierInfoForSlot(unsigned argIndex) const;
-
+  
+  /// \brief Retrieve the name at a given position in the selector.
+  ///
+  /// \param argIndex The index for which we want to retrieve the name.
+  /// This index shall be less than \c getNumArgs() unless this is a keyword
+  /// selector, in which case 0 is the only permissible value.
+  ///
+  /// \returns the name for this slot, which may be the empty string if no
+  /// name was supplied.
+  llvm::StringRef getNameForSlot(unsigned argIndex) const;
+  
   /// getAsString - Derive the full selector name (e.g. "foo:bar:") and return
   /// it as an std::string.
   std::string getAsString() const;

diff --git a/lib/AST/DeclarationName.cpp b/lib/AST/DeclarationName.cpp
index ac3989bbb8ebfc1259b50ade2cccbf44f4129597..cef54e97c93a2326b31837383ef6da02bfaca422 100644 (file)
--- a/lib/AST/DeclarationName.cpp
+++ b/lib/AST/DeclarationName.cpp
@@ -94,10 +94,8 @@ int DeclarationName::compare(DeclarationName LHS, DeclarationName RHS) {
     Selector RHSSelector = RHS.getObjCSelector();
     unsigned LN = LHSSelector.getNumArgs(), RN = RHSSelector.getNumArgs();
     for (unsigned I = 0, N = std::min(LN, RN); I != N; ++I) {
-      IdentifierInfo *LHSId = LHSSelector.getIdentifierInfoForSlot(I);
-      IdentifierInfo *RHSId = RHSSelector.getIdentifierInfoForSlot(I);
-        
-      switch (LHSId->getName().compare(RHSId->getName())) {
+      switch (LHSSelector.getNameForSlot(I).compare(
+                                               RHSSelector.getNameForSlot(I))) {
       case -1: return true;
       case 1: return false;
       default: break;

diff --git a/lib/AST/StmtPrinter.cpp b/lib/AST/StmtPrinter.cpp
index a67e269790ceadc59bcb3ee1e2e13408df070b3b..1cdd2208814197add72a9f0e0f3be9c05c516c27 100644 (file)
--- a/lib/AST/StmtPrinter.cpp
+++ b/lib/AST/StmtPrinter.cpp
@@ -1311,7 +1311,7 @@ void StmtPrinter::VisitObjCMessageExpr(ObjCMessageExpr *Mess) {
   OS << ' ';
   Selector selector = Mess->getSelector();
   if (selector.isUnarySelector()) {
-    OS << selector.getIdentifierInfoForSlot(0)->getName();
+    OS << selector.getNameForSlot(0);
   } else {
     for (unsigned i = 0, e = Mess->getNumArgs(); i != e; ++i) {
       if (i < selector.getNumArgs()) {

diff --git a/lib/Basic/IdentifierTable.cpp b/lib/Basic/IdentifierTable.cpp
index 48a5f49914b2a536b1bdd6eab550250c07c9e198..ef11d658ed9e10ef3bcebd2006c4ab1805b888d9 100644 (file)
--- a/lib/Basic/IdentifierTable.cpp
+++ b/lib/Basic/IdentifierTable.cpp
@@ -326,6 +326,11 @@ IdentifierInfo *Selector::getIdentifierInfoForSlot(unsigned argIndex) const {
   return SI->getIdentifierInfoForSlot(argIndex);
 }
 
+llvm::StringRef Selector::getNameForSlot(unsigned int argIndex) const {
+  IdentifierInfo *II = getIdentifierInfoForSlot(argIndex);
+  return II? II->getName() : llvm::StringRef();
+}
+
 std::string MultiKeywordSelector::getName() const {
   llvm::SmallString<256> Str;
   llvm::raw_svector_ostream OS(Str);

diff --git a/lib/Sema/SemaCodeComplete.cpp b/lib/Sema/SemaCodeComplete.cpp
index 703734e247364bc82b98ac3d39a92b5afca17651..a65d5fd6fc705e0514948c019d67f2660438d797 100644 (file)
--- a/lib/Sema/SemaCodeComplete.cpp
+++ b/lib/Sema/SemaCodeComplete.cpp
@@ -2391,11 +2391,11 @@ CodeCompletionResult::CreateCodeCompletionString(Sema &S,
     Selector Sel = Method->getSelector();
     if (Sel.isUnarySelector()) {
       Result.AddTypedTextChunk(Result.getAllocator().CopyString(
-                                  Sel.getIdentifierInfoForSlot(0)->getName()));
+                                  Sel.getNameForSlot(0)));
       return Result.TakeString();
     }
 
-    std::string SelName = Sel.getIdentifierInfoForSlot(0)->getName().str();
+    std::string SelName = Sel.getNameForSlot(0).str();
     SelName += ':';
     if (StartParameter == 0)
       Result.AddTypedTextChunk(Result.getAllocator().CopyString(SelName));
@@ -4531,10 +4531,10 @@ static ObjCMethodDecl *AddSuperSendCompletion(Sema &S, bool NeedSuperKeyword,
   if (Sel.isUnarySelector()) {
     if (NeedSuperKeyword)
       Builder.AddTextChunk(Builder.getAllocator().CopyString(
-                                  Sel.getIdentifierInfoForSlot(0)->getName()));
+                                  Sel.getNameForSlot(0)));
     else
       Builder.AddTypedTextChunk(Builder.getAllocator().CopyString(
-                                   Sel.getIdentifierInfoForSlot(0)->getName()));
+                                   Sel.getNameForSlot(0)));
   } else {
     ObjCMethodDecl::param_iterator CurP = CurMethod->param_begin();
     for (unsigned I = 0, N = Sel.getNumArgs(); I != N; ++I, ++CurP) {
@@ -4544,17 +4544,17 @@ static ObjCMethodDecl *AddSuperSendCompletion(Sema &S, bool NeedSuperKeyword,
       if (I < NumSelIdents)
         Builder.AddInformativeChunk(
                    Builder.getAllocator().CopyString(
-                      Sel.getIdentifierInfoForSlot(I)->getName().str() + ":"));
+                                                 Sel.getNameForSlot(I) + ":"));
       else if (NeedSuperKeyword || I > NumSelIdents) {
         Builder.AddTextChunk(
                  Builder.getAllocator().CopyString(
-                      Sel.getIdentifierInfoForSlot(I)->getName().str() + ":"));
+                                                  Sel.getNameForSlot(I) + ":"));
         Builder.AddPlaceholderChunk(Builder.getAllocator().CopyString(
                                          (*CurP)->getIdentifier()->getName()));
       } else {
         Builder.AddTypedTextChunk(
                   Builder.getAllocator().CopyString(
-                      Sel.getIdentifierInfoForSlot(I)->getName().str() + ":"));
+                                                  Sel.getNameForSlot(I) + ":"));
         Builder.AddPlaceholderChunk(Builder.getAllocator().CopyString(
                                          (*CurP)->getIdentifier()->getName())); 
       }
@@ -5002,7 +5002,7 @@ void Sema::CodeCompleteObjCSelector(Scope *S, IdentifierInfo **SelIdents,
     CodeCompletionBuilder Builder(Results.getAllocator());
     if (Sel.isUnarySelector()) {
       Builder.AddTypedTextChunk(Builder.getAllocator().CopyString(
-                                   Sel.getIdentifierInfoForSlot(0)->getName()));
+                                                       Sel.getNameForSlot(0)));
       Results.AddResult(Builder.TakeString());
       continue;
     }
@@ -5017,7 +5017,7 @@ void Sema::CodeCompleteObjCSelector(Scope *S, IdentifierInfo **SelIdents,
         }
       }
       
-      Accumulator += Sel.getIdentifierInfoForSlot(I)->getName().str();
+      Accumulator += Sel.getNameForSlot(I).str();
       Accumulator += ':';
     }
     Builder.AddTypedTextChunk(Builder.getAllocator().CopyString( Accumulator));
@@ -6115,7 +6115,7 @@ void Sema::CodeCompleteObjCMethodDecl(Scope *S,
 
     // Add the first part of the selector to the pattern.
     Builder.AddTypedTextChunk(Builder.getAllocator().CopyString(
-                                   Sel.getIdentifierInfoForSlot(0)->getName()));
+                                                       Sel.getNameForSlot(0)));
 
     // Add parameters to the pattern.
     unsigned I = 0;
@@ -6128,9 +6128,7 @@ void Sema::CodeCompleteObjCMethodDecl(Scope *S,
       else if (I < Sel.getNumArgs()) {
         Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
         Builder.AddTypedTextChunk(
-                          Builder.getAllocator().CopyString(
-                                   (Sel.getIdentifierInfoForSlot(I)->getName()
-                                    + ":").str()));
+                Builder.getAllocator().CopyString(Sel.getNameForSlot(I) + ":"));
       } else
         break;
 

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index d5e6e77c3e57927b2e287cd5ad53fbb6f192f90b..7e6eee7fb7b938afc4c8431f06bb9ae8f667925c 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -9487,13 +9487,11 @@ void Sema::DiagnoseAssignmentAsCondition(Expr *E) {
       Selector Sel = ME->getSelector();
 
       // self = [<foo> init...]
-      if (isSelfExpr(Op->getLHS())
-          && Sel.getIdentifierInfoForSlot(0)->getName().startswith("init"))
+      if (isSelfExpr(Op->getLHS()) && Sel.getNameForSlot(0).startswith("init"))
         diagnostic = diag::warn_condition_is_idiomatic_assignment;
 
       // <foo> = [<bar> nextObject]
-      else if (Sel.isUnarySelector() &&
-               Sel.getIdentifierInfoForSlot(0)->getName() == "nextObject")
+      else if (Sel.isUnarySelector() && Sel.getNameForSlot(0) == "nextObject")
         diagnostic = diag::warn_condition_is_idiomatic_assignment;
     }
 

diff --git a/test/Index/complete-method-decls.m b/test/Index/complete-method-decls.m
index 92208910e04cce6c2e8484eee793d9f4d99eca5d..2ab119795347d5f37776a19fb3f138042c3b1d36 100644 (file)
--- a/test/Index/complete-method-decls.m
+++ b/test/Index/complete-method-decls.m
@@ -60,6 +60,14 @@
 - (oneway void)method:(in id x);
 @end
 
+@interface Gaps
+- (void)method:(int)x :(int)y;
+@end
+
+@implementation Gaps
+- (void)method:(int)x :(int)y;
+@end
+
 // RUN: c-index-test -code-completion-at=%s:17:3 %s | FileCheck -check-prefix=CHECK-CC1 %s
 // CHECK-CC1: ObjCInstanceMethodDecl:{LeftParen (}{Text id}{RightParen )}{TypedText abc}
 // CHECK-CC1: ObjCInstanceMethodDecl:{LeftParen (}{Text int}{RightParen )}{TypedText getInt}
@@ -163,3 +171,6 @@
 // RUN: c-index-test -code-completion-at=%s:5:4 %s | FileCheck -check-prefix=CHECK-IBACTION %s
 // CHECK-IBACTION: NotImplemented:{TypedText IBAction}{RightParen )}{Placeholder selector}{Colon :}{LeftParen (}{Text id}{RightParen )}{Text sender} (40)
 
+// <rdar://problem/8939352>
+// RUN: c-index-test -code-completion-at=%s:68:9 %s | FileCheck -check-prefix=CHECK-8939352 %s
+// CHECK-8939352: ObjCInstanceMethodDecl:{TypedText method}{TypedText :}{LeftParen (}{Text int}{RightParen )}{Text x}{HorizontalSpace  }{TypedText :}{LeftParen (}{Text int}{RightParen )}{Text y} (40)

diff --git a/test/SemaObjC/self-assign.m b/test/SemaObjC/self-assign.m
new file mode 100644 (file)
index 0000000..f05b028
--- /dev/null
+++ b/test/SemaObjC/self-assign.m
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+@interface A
+@end
+
+@implementation A
+- (id):(int)x :(int)y {
+    int z;
+    // <rdar://problem/8939352>
+    if (self = [self :x :y]) {} // expected-warning{{using the result of an assignment as a condition without parentheses}} \
+    // expected-note{{use '==' to turn this assignment into an equality comparison}} \
+    // expected-note{{place parentheses around the assignment to silence this warning}}
+    return self;
+}
+@end