r276020 - [SemaObjC] Improve ObjCDictionaryLiteral and ObjCArryLiteral diagnostics

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Tue Jul 19 15:21:18 2016
New Revision: 276020

URL: http://llvm.org/viewvc/llvm-project?rev=276020&view=rev
Log:
[SemaObjC] Improve ObjCDictionaryLiteral and ObjCArryLiteral diagnostics

Sema actions on ObjCDictionaryLiteral and ObjCArryLiteral are currently
done as a side-effect of Sema upon parent expressions, which incurs of
delayed typo corrections for such literals to be performed by TypoTransforms
upon the ObjCDictionaryLiteral and ObjCArryLiteral themselves instead of
its elements individually.

This is specially bad because it was not designed to act on several
elements; searching through all possible combinations of corrections for
several elements is very expensive. Additionally, when one of the
elements has no correction candidate, we still explore all options and
at the end emit no typo corrections whatsoever.

Do the proper sema actions by acting on each element alone during appropriate
literal parsing time to get proper diagonistics and decent compile time
behavior.

Differential Revision: http://reviews.llvm.org/D22183

rdar://problem/21046678

Modified:
    cfe/trunk/lib/Parse/ParseObjc.cpp
    cfe/trunk/test/SemaObjC/objc-array-literal.m
    cfe/trunk/test/SemaObjC/objc-dictionary-literal.m

Modified: cfe/trunk/lib/Parse/ParseObjc.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseObjc.cpp?rev=276020&r1=276019&r2=276020&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseObjc.cpp (original)
+++ cfe/trunk/lib/Parse/ParseObjc.cpp Tue Jul 19 15:21:18 2016
@@ -3416,6 +3416,7 @@ ExprResult Parser::ParseObjCArrayLiteral
   ExprVector ElementExprs;                   // array elements.
   ConsumeBracket(); // consume the l_square.
 
+  bool HasInvalidEltExpr = false;
   while (Tok.isNot(tok::r_square)) {
     // Parse list of array element expressions (all must be id types).
     ExprResult Res(ParseAssignmentExpression());
@@ -3427,11 +3428,15 @@ ExprResult Parser::ParseObjCArrayLiteral
       return Res;
     }    
     
+    Res = Actions.CorrectDelayedTyposInExpr(Res.get());
+    if (Res.isInvalid())
+      HasInvalidEltExpr = true;
+
     // Parse the ellipsis that indicates a pack expansion.
     if (Tok.is(tok::ellipsis))
       Res = Actions.ActOnPackExpansion(Res.get(), ConsumeToken());    
     if (Res.isInvalid())
-      return true;
+      HasInvalidEltExpr = true;
 
     ElementExprs.push_back(Res.get());
 
@@ -3442,6 +3447,10 @@ ExprResult Parser::ParseObjCArrayLiteral
                                                             << tok::comma);
   }
   SourceLocation EndLoc = ConsumeBracket(); // location of ']'
+
+  if (HasInvalidEltExpr)
+    return ExprError();
+
   MultiExprArg Args(ElementExprs);
   return Actions.BuildObjCArrayLiteral(SourceRange(AtLoc, EndLoc), Args);
 }
@@ -3449,6 +3458,7 @@ ExprResult Parser::ParseObjCArrayLiteral
 ExprResult Parser::ParseObjCDictionaryLiteral(SourceLocation AtLoc) {
   SmallVector<ObjCDictionaryElement, 4> Elements; // dictionary elements.
   ConsumeBrace(); // consume the l_square.
+  bool HasInvalidEltExpr = false;
   while (Tok.isNot(tok::r_brace)) {
     // Parse the comma separated key : value expressions.
     ExprResult KeyExpr;
@@ -3478,7 +3488,15 @@ ExprResult Parser::ParseObjCDictionaryLi
       return ValueExpr;
     }
     
-    // Parse the ellipsis that designates this as a pack expansion.
+    // Check the key and value for possible typos
+    KeyExpr = Actions.CorrectDelayedTyposInExpr(KeyExpr.get());
+    ValueExpr = Actions.CorrectDelayedTyposInExpr(ValueExpr.get());
+    if (KeyExpr.isInvalid() || ValueExpr.isInvalid())
+      HasInvalidEltExpr = true;
+
+    // Parse the ellipsis that designates this as a pack expansion. Do not
+    // ActOnPackExpansion here, leave it to template instantiation time where
+    // we can get better diagnostics.
     SourceLocation EllipsisLoc;
     if (getLangOpts().CPlusPlus)
       TryConsumeToken(tok::ellipsis, EllipsisLoc);
@@ -3495,6 +3513,9 @@ ExprResult Parser::ParseObjCDictionaryLi
                                                             << tok::comma);
   }
   SourceLocation EndLoc = ConsumeBrace();
+
+  if (HasInvalidEltExpr)
+    return ExprError();
   
   // Create the ObjCDictionaryLiteral.
   return Actions.BuildObjCDictionaryLiteral(SourceRange(AtLoc, EndLoc),

Modified: cfe/trunk/test/SemaObjC/objc-array-literal.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/objc-array-literal.m?rev=276020&r1=276019&r2=276020&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/objc-array-literal.m (original)
+++ cfe/trunk/test/SemaObjC/objc-array-literal.m Tue Jul 19 15:21:18 2016
@@ -67,3 +67,11 @@ id radar15147688() {
   x = @[ @"stuff", @"hello" "world"]; // expected-warning {{concatenated NSString literal for an NSArray expression}}
   return x;
 }
+
+enum XXXYYYZZZType { XXXYYYZZZTypeAny }; // expected-note {{'XXXYYYZZZTypeAny' declared here}}
+void foo() {
+  NSArray *array = @[
+    @(XXXYYYZZZTypeA),                 // expected-error {{use of undeclared identifier 'XXXYYYZZZTypeA'; did you mean 'XXXYYYZZZTypeAny'}}
+    @(XXXYYYZZZTypeSomethingSomething) // expected-error {{use of undeclared identifier 'XXXYYYZZZTypeSomethingSomething'}}
+  ];
+}

Modified: cfe/trunk/test/SemaObjC/objc-dictionary-literal.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/objc-dictionary-literal.m?rev=276020&r1=276019&r2=276020&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/objc-dictionary-literal.m (original)
+++ cfe/trunk/test/SemaObjC/objc-dictionary-literal.m Tue Jul 19 15:21:18 2016
@@ -63,3 +63,10 @@ int main() {
 	return 0;
 }
 
+enum XXXYYYZZZType { XXXYYYZZZTypeAny }; // expected-note {{'XXXYYYZZZTypeAny' declared here}}
+void foo() {
+  NSDictionary *d = @{
+    @"A" : @(XXXYYYZZZTypeA), // expected-error {{use of undeclared identifier 'XXXYYYZZZTypeA'; did you mean 'XXXYYYZZZTypeAny'}}
+    @"F" : @(XXXYYYZZZTypeSomethingSomething), // expected-error {{use of undeclared identifier 'XXXYYYZZZTypeSomethingSomething'}}
+  };
+}