rsmith added a comment.
Please produce patches with more lines of context in future; phabricator only
lets us comment on lines that are included in the patch, and in this case some
of the relevant parts of the function are not in the context. (The equivalent
of diff -U1000 is a common approach for this.)
Taking a step back, I wonder whether we have the right strategy overall for
code completion within in-class //mem-initializer-list//s. The code after the
code completion token is quite plausibly not even brace-balanced, in a case
where you're writing a new constructor. Consider this completion:
struct Foo {
Foo() : some_long_x(0), some_|
int some_long_x, some_long_y;
};
Here, we ought to be able to complete "some_long_y", but we need to recognize
that the next token is not part of the function definition in order to do that
(and then we need to not try to consume a function body once we're done with
the initializer). And conversely when completing here:
struct Foo {
Foo() : some_long_x(0), some_| {}
int some_long_x, some_long_y;
};
... we need to recognize that we //do// have a function body so that we can
parse the members to find out what names we should complete.
However, this patch is an incremental improvement over what we already have, so
I'm happy to go in this direction for now.
================
Comment at: lib/Parse/ParseCXXInlineMethods.cpp:840
@@ +839,3 @@
+
+ if (Tok.isOneOf(tok::identifier, tok::kw_template)) {
+ Toks.push_back(Tok);
----------------
Can you delete the check for `kw_template` here? We handle the `template`
keyword above, and any time we actually hit this case we would expect
`template` to be followed by one of `::`, `(`, or `{`, which makes no sense.
================
Comment at: lib/Parse/ParseCXXInlineMethods.cpp:847-851
@@ -849,2 +846,7 @@
} while (Tok.is(tok::coloncolon));
+ if (Tok.is(tok::code_completion)) {
+ Toks.push_back(Tok);
+ ConsumeCodeCompletionToken();
+ }
+
----------------
You should probably also handle the case of a comma immediately after the code
completion token, for completions like this:
struct A {
A() : new_mem|, existing_member() {}
int new_member, existing_member;
};
... and likewise the case where the completion token is followed by an
identifier, a `::`, or a `decltype`, for completions like this:
struct A {
A() : new_mem| existing_member() {}
int new_member, existing_member;
};
In all those cases, I think you can just `continue` to pick up the rest of the
initializers after the code completion token. (And if you `continue` from here
in those cases, I think you can remove the handling of the code_completion
token up on line 835, since this codepath will do the right thing in all those
cases.)
================
Comment at: lib/Parse/ParseCXXInlineMethods.cpp:894-897
@@ +893,6 @@
+
+ const Token &PreviousToken = Toks[Toks.size() - 2];
+ if (!MightBeTemplateArgument &&
+ !PreviousToken.isOneOf(tok::identifier, tok::greater,
+ tok::greatergreater)) {
+ // Most likely the start of a function body rather than the start of a
----------------
Please add a comment here indicating that if the previous token is not one of
these kinds, we've encountered an error (because this //mem-initializer// is
missing its initializer). This is not really obvious, and it's important
because that's what makes it correct to use a heuristic to guess whether we've
got a function body next.
https://reviews.llvm.org/D21502
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
