On Fri, May 23, 2014 at 10:47 AM, Serge Pavlov <[email protected]> wrote:
> > > > 2014-05-23 6:43 GMT+07:00 Richard Smith <[email protected]>: > > --- lib/Parse/ParseDecl.cpp >> +++ lib/Parse/ParseDecl.cpp >> @@ -2706,7 +2706,7 @@ >> case tok::identifier: { >> // In C++, check to see if this is a scope specifier like >> foo::bar::, if >> // so handle it as such. This is important for ctor parsing. >> - if (getLangOpts().CPlusPlus) { >> + if (getLangOpts().CPlusPlus && !DS.hasTypeSpecifier()) { >> >> Please instead swap over this 'if' and the next one. >> >> > Removed unneded check. > > >> >> @@ -4508,8 +4508,10 @@ >> // Member pointers get special handling, since there's no place for the >> // scope spec in the generic path below. >> if (getLangOpts().CPlusPlus && >> - (Tok.is(tok::coloncolon) || Tok.is(tok::identifier) || >> - Tok.is(tok::annot_cxxscope))) { >> + (Tok.is(tok::coloncolon) || >> + (Tok.is(tok::identifier) && (NextToken().is(tok::coloncolon) || >> + NextToken().is(tok::less))) || >> + Tok.is(tok::annot_cxxscope))) { >> >> I don't think this is quite sufficient. If we have: >> >> struct A { enum E {}; }; >> struct B { >> static const int N = 3; >> A::E : N; >> }; >> >> ... I think we might treat the : as a :: typo. Instead, drop a >> ColonProtectionRAII object into this 'if', with a comment saying you're >> doing it for bitfields, if you're in a member context. >> > > Not sure if I undestand correctly. This piece of code is correcly parsed > as unnamed bit field of enum type. Testcase lacked for tests on unnamed > bitfields, they are added now. > You may need to try a bit harder to get this to misparse, but I think it will. The point is that we're not protecting colons when we parse the scope specifier, and the check: - (Tok.is(tok::coloncolon) || Tok.is(tok::identifier) || - Tok.is(tok::annot_cxxscope))) { + (Tok.is(tok::coloncolon) || + (Tok.is(tok::identifier) && (NextToken().is(tok::coloncolon) || + NextToken().is(tok::less))) || + Tok.is(tok::annot_cxxscope))) { only protects a colon at the start, not a colon *after* the scope. Here's another attempt to get this to fail: const int m = 4; struct A { struct n { int m; }; int A::n : m; }; This code is valid with -fms-extensions, but I think you might correct the ':' to a '::' here. What I'm suggesting is that you revert the above change and add in colon-protection around the parsing of the scope specifier. Other than that, this patch looks great! On Fri, May 9, 2014 at 4:17 AM, Serge Pavlov <[email protected]> wrote: >> >>> Hi Richard, >>> >>> Could you please review this fix? >>> Thank you. >>> >>> --Serge >>> >>> Recognize additional cases, when '::' is mistyped as ':' and provide >>> descriptive diagnostics for them. >>> This is a fix to RP18587 - colons have too much protection in >>> member-declarations >>> >>> http://reviews.llvm.org/D3653 >>> >>> Files: >>> lib/Parse/ParseDecl.cpp >>> lib/Parse/ParseDeclCXX.cpp >>> test/SemaCXX/nested-name-spec.cpp >>> >>> >> > > > -- > Thanks, > --Serge >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
