Only check NSArray/NSDictionary boxing method params once.

Once we've found a "good" method, we don't need to check its argument types
again. (Even if we might have later found a "bad" method, we were already
caching the method we first looked up.)

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

lib/Sema/SemaExprObjC.cpp

@@ -655,35 +655,38 @@ ExprResult Sema::BuildObjCArrayLiteral(SourceRange SR, MultiExprArg Elements) {
     if (!validateBoxingMethod(*this, SR.getBegin(), NSArrayDecl, Sel, Method))
       return ExprError();
 
+    // Dig out the type that all elements should be converted to.
+    QualType T = Method->param_begin()[0]->getType();
+    const PointerType *PtrT = T->getAs<PointerType>();
+    if (!PtrT || 
+        !Context.hasSameUnqualifiedType(PtrT->getPointeeType(), IdT)) {
+      Diag(SR.getBegin(), diag::err_objc_literal_method_sig)
+        << Sel;
+      Diag(Method->param_begin()[0]->getLocation(),
+           diag::note_objc_literal_method_param)
+        << 0 << T 
+        << Context.getPointerType(IdT.withConst());
+      return ExprError();
+    }
+  
+    // Check that the 'count' parameter is integral.
+    if (!Method->param_begin()[1]->getType()->isIntegerType()) {
+      Diag(SR.getBegin(), diag::err_objc_literal_method_sig)
+        << Sel;
+      Diag(Method->param_begin()[1]->getLocation(),
+           diag::note_objc_literal_method_param)
+        << 1 
+        << Method->param_begin()[1]->getType()
+        << "integral";
+      return ExprError();
+    }
+
+    // We've found a good +arrayWithObjects:count: method. Save it!
     ArrayWithObjectsMethod = Method;
   }
 
-  // Dig out the type that all elements should be converted to.
+  QualType ObjectsType = ArrayWithObjectsMethod->param_begin()[0]->getType();
-  QualType T = ArrayWithObjectsMethod->param_begin()[0]->getType();
+  QualType RequiredType = ObjectsType->castAs<PointerType>()->getPointeeType();
-  const PointerType *PtrT = T->getAs<PointerType>();
-  if (!PtrT || 
-      !Context.hasSameUnqualifiedType(PtrT->getPointeeType(), IdT)) {
-    Diag(SR.getBegin(), diag::err_objc_literal_method_sig)
-      << ArrayWithObjectsMethod->getSelector();
-    Diag(ArrayWithObjectsMethod->param_begin()[0]->getLocation(),
-         diag::note_objc_literal_method_param)
-      << 0 << T 
-      << Context.getPointerType(IdT.withConst());
-    return ExprError();
-  }
-  T = PtrT->getPointeeType();
-  
-  // Check that the 'count' parameter is integral.
-  if (!ArrayWithObjectsMethod->param_begin()[1]->getType()->isIntegerType()) {
-    Diag(SR.getBegin(), diag::err_objc_literal_method_sig)
-      << ArrayWithObjectsMethod->getSelector();
-    Diag(ArrayWithObjectsMethod->param_begin()[1]->getLocation(),
-         diag::note_objc_literal_method_param)
-      << 1 
-      << ArrayWithObjectsMethod->param_begin()[1]->getType()
-      << "integral";
-    return ExprError();
-  }
 
   // Check that each of the elements provided is valid in a collection literal,
   // performing conversions as necessary.
@@ -691,7 +694,7 @@ ExprResult Sema::BuildObjCArrayLiteral(SourceRange SR, MultiExprArg Elements) {
   for (unsigned I = 0, N = Elements.size(); I != N; ++I) {
     ExprResult Converted = CheckObjCCollectionLiteralElement(*this,
                                                              ElementsBuffer[I],
-                                                             T);
+                                                             RequiredType);
     if (Converted.isInvalid())
       return ExprError();
     
@@ -781,73 +784,76 @@ ExprResult Sema::BuildObjCDictionaryLiteral(SourceRange SR,
                               Method))
        return ExprError();
 
-    DictionaryWithObjectsMethod = Method;
+    // Dig out the type that all values should be converted to.
-  }
+    QualType ValueT = Method->param_begin()[0]->getType();
-
+    const PointerType *PtrValue = ValueT->getAs<PointerType>();
-  // Dig out the type that all values should be converted to.
+    if (!PtrValue || 
-  QualType ValueT =  DictionaryWithObjectsMethod->param_begin()[0]->getType();
+        !Context.hasSameUnqualifiedType(PtrValue->getPointeeType(), IdT)) {
-  const PointerType *PtrValue = ValueT->getAs<PointerType>();
-  if (!PtrValue || 
-      !Context.hasSameUnqualifiedType(PtrValue->getPointeeType(), IdT)) {
-    Diag(SR.getBegin(), diag::err_objc_literal_method_sig)
-      << DictionaryWithObjectsMethod->getSelector();
-    Diag(DictionaryWithObjectsMethod->param_begin()[0]->getLocation(),
-         diag::note_objc_literal_method_param)
-      << 0 << ValueT
-      << Context.getPointerType(IdT.withConst());
-    return ExprError();
-  }
-  ValueT = PtrValue->getPointeeType();
-
-  // Dig out the type that all keys should be converted to.
-  QualType KeyT = DictionaryWithObjectsMethod->param_begin()[1]->getType();
-  const PointerType *PtrKey = KeyT->getAs<PointerType>();
-  if (!PtrKey || 
-      !Context.hasSameUnqualifiedType(PtrKey->getPointeeType(),
-                                      IdT)) {
-    bool err = true;
-    if (PtrKey) {
-      if (QIDNSCopying.isNull()) {
-        // key argument of selector is id<NSCopying>?
-        if (ObjCProtocolDecl *NSCopyingPDecl =
-            LookupProtocol(&Context.Idents.get("NSCopying"), SR.getBegin())) {
-          ObjCProtocolDecl *PQ[] = {NSCopyingPDecl};
-          QIDNSCopying = 
-            Context.getObjCObjectType(Context.ObjCBuiltinIdTy,
-                                      (ObjCProtocolDecl**) PQ,1);
-          QIDNSCopying = Context.getObjCObjectPointerType(QIDNSCopying);
-        }
-      }
-      if (!QIDNSCopying.isNull())
-        err = !Context.hasSameUnqualifiedType(PtrKey->getPointeeType(),
-                                              QIDNSCopying);
-    }
-    
-    if (err) {
       Diag(SR.getBegin(), diag::err_objc_literal_method_sig)
-        << DictionaryWithObjectsMethod->getSelector();
+        << Sel;
-      Diag(DictionaryWithObjectsMethod->param_begin()[1]->getLocation(),
+      Diag(Method->param_begin()[0]->getLocation(),
            diag::note_objc_literal_method_param)
-        << 1 << KeyT
+        << 0 << ValueT
         << Context.getPointerType(IdT.withConst());
       return ExprError();
     }
-  }
-  KeyT = PtrKey->getPointeeType();
 
-  // Check that the 'count' parameter is integral.
+    // Dig out the type that all keys should be converted to.
-  if (!DictionaryWithObjectsMethod->param_begin()[2]->getType()
+    QualType KeyT = Method->param_begin()[1]->getType();
-                                                            ->isIntegerType()) {
+    const PointerType *PtrKey = KeyT->getAs<PointerType>();
-    Diag(SR.getBegin(), diag::err_objc_literal_method_sig)
+    if (!PtrKey || 
-      << DictionaryWithObjectsMethod->getSelector();
+        !Context.hasSameUnqualifiedType(PtrKey->getPointeeType(),
-    Diag(DictionaryWithObjectsMethod->param_begin()[2]->getLocation(),
+                                        IdT)) {
-         diag::note_objc_literal_method_param)
+      bool err = true;
-      << 2
+      if (PtrKey) {
-      << DictionaryWithObjectsMethod->param_begin()[2]->getType()
+        if (QIDNSCopying.isNull()) {
-      << "integral";
+          // key argument of selector is id<NSCopying>?
-    return ExprError();
+          if (ObjCProtocolDecl *NSCopyingPDecl =
+              LookupProtocol(&Context.Idents.get("NSCopying"), SR.getBegin())) {
+            ObjCProtocolDecl *PQ[] = {NSCopyingPDecl};
+            QIDNSCopying = 
+              Context.getObjCObjectType(Context.ObjCBuiltinIdTy,
+                                        (ObjCProtocolDecl**) PQ,1);
+            QIDNSCopying = Context.getObjCObjectPointerType(QIDNSCopying);
+          }
+        }
+        if (!QIDNSCopying.isNull())
+          err = !Context.hasSameUnqualifiedType(PtrKey->getPointeeType(),
+                                                QIDNSCopying);
+      }
+    
+      if (err) {
+        Diag(SR.getBegin(), diag::err_objc_literal_method_sig)
+          << Sel;
+        Diag(Method->param_begin()[1]->getLocation(),
+             diag::note_objc_literal_method_param)
+          << 1 << KeyT
+          << Context.getPointerType(IdT.withConst());
+        return ExprError();
+      }
+    }
+
+    // Check that the 'count' parameter is integral.
+    QualType CountType = Method->param_begin()[2]->getType();
+    if (!CountType->isIntegerType()) {
+      Diag(SR.getBegin(), diag::err_objc_literal_method_sig)
+        << Sel;
+      Diag(Method->param_begin()[2]->getLocation(),
+           diag::note_objc_literal_method_param)
+        << 2 << CountType
+        << "integral";
+      return ExprError();
+    }
+
+    // We've found a good +dictionaryWithObjects:keys:count: method; save it!
+    DictionaryWithObjectsMethod = Method;
   }
 
+  QualType ValuesT = DictionaryWithObjectsMethod->param_begin()[0]->getType();
+  QualType ValueT = ValuesT->castAs<PointerType>()->getPointeeType();
+  QualType KeysT = DictionaryWithObjectsMethod->param_begin()[1]->getType();
+  QualType KeyT = KeysT->castAs<PointerType>()->getPointeeType();
+
   // Check that each of the keys and values provided is valid in a collection 
   // literal, performing conversions as necessary.
   bool HasPackExpansions = false;

test/SemaObjC/objc-literal-sig.m

@@ -16,30 +16,36 @@ typedef _Bool BOOL;
 + (NSNumber *)numberWithUnsignedLongLong:(unsigned long long)value;
 + (NSNumber *)numberWithFloat:(float)value;
 + (NSNumber *)numberWithDouble:(double)value;
-+ (int)numberWithBool:(BOOL)value; // expected-note{{method returns unexpected type 'int' (should be an object type)}}
++ (int)numberWithBool:(BOOL)value; // expected-note 2 {{method returns unexpected type 'int' (should be an object type)}}
 @end
 
 @interface NSString
-+ (char)stringWithUTF8String:(const char *)value; // expected-note{{method returns unexpected type 'char' (should be an object type)}}
++ (char)stringWithUTF8String:(const char *)value; // expected-note 2 {{method returns unexpected type 'char' (should be an object type)}}
 @end
 
 @interface NSArray
 @end
 
 @interface NSArray (NSArrayCreation)
-+ (id)arrayWithObjects:(const int [])objects // expected-note{{first parameter has unexpected type 'const int *' (should be 'const id *')}}
++ (id)arrayWithObjects:(const int [])objects // expected-note 2 {{first parameter has unexpected type 'const int *' (should be 'const id *')}}
                  count:(unsigned long)cnt;
 @end
 
 @interface NSDictionary
 + (id)dictionaryWithObjects:(const id [])objects
-                    forKeys:(const int [])keys // expected-note{{second parameter has unexpected type 'const int *' (should be 'const id *')}}
+                    forKeys:(const int [])keys // expected-note 2 {{second parameter has unexpected type 'const int *' (should be 'const id *')}}
                       count:(unsigned long)cnt;
 @end
 
+// All tests are doubled to make sure that a bad method is not saved
+// and then used un-checked.
 void test_sig() {
+  (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
   id array = @[ @17 ]; // expected-error{{literal construction method 'arrayWithObjects:count:' has incompatible signature}}
+  id array2 = @[ @17 ]; // expected-error{{literal construction method 'arrayWithObjects:count:' has incompatible signature}}
   id dict = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible signature}}
+  id dict2 = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible signature}}
   id str = @("hello"); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
+  id str2 = @("hello"); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
 }