Thanks! Here is the updated patch that: - Enable ParseImplicitInt to pass attributes to caller. - Handle alignment specifiers as well. - Fix comments and variable naming Sean pointed out. - Update test case.
Cheers Michael On Sun, Nov 18, 2012 at 7:00 PM, Richard Smith <[email protected]>wrote: > Index: lib/Parse/ParseDecl.cpp > =================================================================== > --- lib/Parse/ParseDecl.cpp (revision 168292) > +++ lib/Parse/ParseDecl.cpp (working copy) > @@ -1923,12 +1923,13 @@ > << TokenName << TagName; > } > > + ParsedAttributesWithRange attrs(AttrFactory); > // Parse this as a tag as if the missing tag were present. > if (TagKind == tok::kw_enum) > ParseEnumSpecifier(Loc, DS, TemplateInfo, AS, DSC_normal); > else > ParseClassSpecifier(TagKind, Loc, DS, TemplateInfo, AS, > - /*EnteringContext*/ false, DSC_normal); > + /*EnteringContext*/ false, DSC_normal, attrs); > return true; > } > > It looks like the attributes are being discarded here. This is an > error path, but we have a fix-it so we still need to recover properly. > It'd be fine to add an extra parameter to ParseImplicitInt for this. > > @@ -1234,12 +1235,24 @@ > // - If we have 'struct foo;', then this is either a forward > declaration > // or a friend declaration, which have to be treated differently. > // - Otherwise we have something like 'struct foo xyz', a reference. > + // When start parsing from here, we also take misplaced C++11 > attributes > + // specifiers into consideration by detecting the following cases: > + // - attributes follow class-name: > + // struct foo [[]] {}; > + // - attributes appear before or after 'final': > + // struct foo [[]] final [[]] {}; > + // This allow we produce better diagnostics. > > Something funny seems to have happened to the grammar in this comment. > > @@ -1261,6 +1274,30 @@ > // Okay, this is a class definition. > TUK = Sema::TUK_Definition; > } > + } else if (isCXX0XFinalKeyword() && (NextToken().is(tok::l_square))) { > + // We can't tell if this is a declaration or definition or reference > + // until we skipped the 'final' and attributes sequences. > > This comment isn't quite right: this can't be a declaration if we have > the 'final' keyword here. > > + do { > + // Skip the attributes without parsing them > + if (NextToken().is(tok::l_square)) { > + ConsumeBracket(); > + ConsumeBracket(); > + SkipUntil(tok::r_square, false); > + SkipUntil(tok::r_square, false); > > The indentation here is off. > > + } > + } while (Tok.is(tok::l_square)); > > It'd be good to handle alignment-specifiers (the other kind of C++11 > attribute-specifier) here. > > On Sun, Nov 18, 2012 at 3:52 PM, Sean Silva <[email protected]> wrote: > > + ParsedAttributesWithRange attrs(AttrFactory); > > > > Variable names should be uppercase, as per the coding standards: > > < > http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly > > > > > > -- Sean Silva > > > > On Sun, Nov 18, 2012 at 5:52 PM, Michael Han <[email protected]> > wrote: > >> Hi Richard, > >> > >> Thanks for the suggestions! > >> > >> Attached updated patch. The parser now will parse C++11 attribute > >> specifiers, in the form of double squares that appear at all possible > >> syntactic locations following class-name in a class specifier (also > before > >> or after 'final' keyword). In case of final keyword, I have not found a > way > >> to classify the nature of the class specifier without using tentative > >> parsing. Also updated test cases. > >> > >> I will look into fix-it in a separate patch. > >> > >> Cheers > >> Michael > >> > >> > >> On Thu, Nov 15, 2012 at 10:32 PM, Richard Smith <[email protected]> > >> wrote: > >>> > >>> On Thu, Nov 15, 2012 at 10:33 AM, Michael Han < > [email protected]> > >>> wrote: > >>> > Hi, > >>> > > >>> > Consider this code > >>> > class [[foo]] c [[foo]]; > >>> > > >>> > Clang generates diagnostic like this: > >>> > error: an attribute list cannot appear here > >>> > class [[foo]] c [[foo]]; > >>> > ^~~~ > >>> > > >>> > I think the first attribute is conforming, and it's the second > attribute > >>> > that should be diagnosed. > >>> > >>> Yes, I agree that it would be better to point the diagnostic at the > >>> second attribute-specifier-seq (and ideally to say that it's in the > >>> wrong place, and offer a fixit...). > >>> > >>> > Attached the patch that fixes this. > >>> > >>> I don't think your approach here is going to work. This is valid: > >>> > >>> class c {}; > >>> class c [[ ]] x; > >>> > >>> I believe your patch would treat 'class c' as a declaration in the > >>> second line, which it is not. > >>> > >>> In order to correctly handle this, I suggest you parse attributes > >>> after the class name as well as before it, before trying to classify > >>> the reference. If you then find that it's a TUK_Reference, return the > >>> attributes you found after the class name to the caller, for them to > >>> handle appropriately. Otherwise, produce a diagnostic indicating that > >>> they were written in the wrong place (offering a fixit would be > >>> awesome...). For a class definition, you should probably look for > >>> attributes both before and after the optional 'final'. > >>> > >>> Thanks for looking into this! > >> > >> > >> > >> _______________________________________________ > >> cfe-commits mailing list > >> [email protected] > >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >> >
parser.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
