bruno added a comment.
Hi Manman,
================
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.
----------------
manmanren wrote:
> 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?
Yes, you're right! Going to update the entire comment to be more accurate
================
Comment at: lib/Parse/ParseObjc.cpp:1720
@@ -1714,3 +1719,3 @@
if (fullTypeArg.isUsable())
typeArgs.push_back(fullTypeArg.get());
else
----------------
manmanren wrote:
> Should we set foundValidTypeId here too?
See the comment below
================
Comment at: lib/Parse/ParseObjc.cpp:1722
@@ -1716,3 +1721,3 @@
else
invalid = true;
} else {
----------------
manmanren wrote:
> Should we emit diagnostics for all cases we set invalid to true?
The initial version of the patch was doing this. However, I could not come up
with a testcase to exercise it, so I left it out. Gonna try harder!
================
Comment at: lib/Parse/ParseObjc.cpp:1726
@@ +1725,3 @@
+ auto *P = Actions.LookupProtocol(identifiers[i], identifierLocs[i]);
+ if (!P) {
+ unknownTypeArgs.push_back(identifiers[i]);
----------------
manmanren wrote:
> Please add comments here: not a type argument, not a protocol, treat it as
> unknown type.
Ok
================
Comment at: lib/Parse/ParseObjc.cpp:1729
@@ +1728,3 @@
+ unknownTypeArgsLoc.push_back(identifierLocs[i]);
+ continue;
+ }
----------------
manmanren wrote:
> Is it necessary to have a "continue" here? Can we restructure this to get rid
> of the "continue"?
Did this to remove some level of nested "if"s which I personally dislike, but
can easily change that
http://reviews.llvm.org/D18997
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
