[PATCH] D18997: [SemaObjC] Properly diagnose type arguments and protocols mix in parameterized classes

[Manman Ren via cfe-commits]

manmanren added inline comments.

================
Comment at: lib/Parse/ParseObjc.cpp:1697
@@ -1696,3 +1696,3 @@
   // We syntactically matched a type argument, so commit to parsing
-  // type arguments.
+  // type arguments. Finding protocol arguments after here is an error.
 
----------------
I am a little confused about the original comment. Have we syntactically matched a type argument? All we have done is trying to parse the single identifiers, right?

================
Comment at: lib/Parse/ParseObjc.cpp:1720
@@ -1714,3 +1719,3 @@
       if (fullTypeArg.isUsable())
         typeArgs.push_back(fullTypeArg.get());
       else
----------------
Should we set foundValidTypeId here too?

================
Comment at: lib/Parse/ParseObjc.cpp:1722
@@ -1716,3 +1721,3 @@
       else
         invalid = true;
     } else {
----------------
Should we emit diagnostics for all cases we set invalid to true?

================
Comment at: lib/Parse/ParseObjc.cpp:1726
@@ +1725,3 @@
+      auto *P = Actions.LookupProtocol(identifiers[i], identifierLocs[i]);
+      if (!P) {
+        unknownTypeArgs.push_back(identifiers[i]);
----------------
Please add comments here: not a type argument, not a protocol, treat it as unknown type.

================
Comment at: lib/Parse/ParseObjc.cpp:1729
@@ +1728,3 @@
+        unknownTypeArgsLoc.push_back(identifierLocs[i]);
+        continue;
+      }
----------------
Is it necessary to have a "continue" here? Can we restructure this to get rid of the "continue"?


http://reviews.llvm.org/D18997