On May 6, 2011, at 6:57 PM, Richard Trieu wrote:
> Moved fix-its from the notes to the error message.
>
> On Fri, May 6, 2011 at 2:20 PM, Richard Trieu <[email protected]> wrote:
> When improperly using "::" to nest namespaces, give a fix-it to correct it.
> Parsing will also recover as if they were properly nested.
>
> namespace foo::bar {
> }
>
> Will result in a fix-it at "::" to change to " { namespace " and an
> additional right bracket at the end.
>
> Patch attached and available through Code Review:
> http://codereview.appspot.com/4452052/
+ // Unless we have "namepsace foo::bar {", just complain about a lack of
'{'.
Typo "namepsace".
+ if (!Tok.is(tok::coloncolon) || !NextToken().is(tok::identifier)) {
+ Diag(Tok, Ident ? diag::err_expected_lbrace :
+ diag::err_expected_ident_lbrace);
+ return 0;
+ }
+ // Otherwise, recover parsing as if the user had typed
+ // "namespace foo { namespace bar {".
+ TentativeParsingAction TPA(*this);
+ SkipUntil(tok::l_brace, /*StopAtSemi*/false);
+ SkipUntil(tok::r_brace, /*StopAtSemi*/false, /*DontConsume*/true);
+ Token rBraceToken = Tok;
+ rBraceToken.setKind(tok::r_brace);
+ PP.EnterToken(rBraceToken);
+ TPA.Revert();
IMO, this isn't robust enough for a Fix-It, because it isn't actually checking
for the presence of the '{' or the '}'. For example, if the "namespace
foo::bar" was at the end of the translation unit, it looks like we could end up
trying to change the end-of-file token to a right-brace.
I think it's fine to use tentative parsing here, but it should only be used to
identify where the '{' and '}' tokens are so we know that this could be a
well-formed namespace definition. Once we've figured that out, I'd prefer that
the code not try to change the token stream. Rather, I think it would be better
just to parse the
namespace foo::bar {
and call the appropriate semantic actions to start the foo namespace, then the
bar namespace, and keep track of the results of each of those calls. Then, when
we see the closing '}', call the appropriate semantic actions to complete those
namespace definitions.
Also, this code is assuming that we're parsing something like
namespace foo::bar {
}
but not
namespace foo::bar::wibble {
}
Why not generalize it to an arbitrary-length identifier-and-:: definitions? If
this were a language feature, we'd want it to support an arbitrary depth.
- Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
