Rebased, reworked a little, and submitted as r192666. Thanks! (And sorry for the delay. Ahem.)
On Mon, Dec 10, 2012 at 9:50 PM, Michael Han <[email protected]>wrote: > Thanks Richard. Patch updated, please take a look. > > I am not sure of passing Disambiguate = true to isCXX11AttributeSpecifier > would make recover better, because the skip logic in this patch explicitly > detects missing brackets (which isCXX11AttributeSpecifier will do when set > to disambiguate mode), and the diagnostic is not emitted based on the > return value of isCXX11AttributeSpecifier. > > For error recovery, I noted there is no existing test cases that covers > ParseCXX11Attributes as well. When an error occurs while parsing > attributes, this patch will skip to next semicolon (and > ParseCXX11Attributes would skip to end of file, which sounds like a bug). > This will disrupt the parser state by eating up tokens which are supposed > to be parsed later. Do you think is it worth to have more heuristics here > to cut off parsing in cases which an attribute is not closed? > > Michael > > > On Fri, Dec 7, 2012 at 4:09 PM, Richard Smith <[email protected]>wrote: > >> On Fri, Dec 7, 2012 at 3:08 PM, Michael Han <[email protected]> >> wrote: >> > Hi, >> > >> > There are a couple of places in parser where we parse attributes, then >> > discard them immediately because these attributes are not allowed to >> appear >> > on certain locations. This patch kills the unnecessary parsing by simply >> > skipping these attributes. >> > >> > The existing attribute tests should cover all cases related to this >> change, >> > with one tweaked test case to cover different combination of C++11 >> attribute >> > syntax ([[ and alignas). >> >> Please use BalancedDelimiterTracker instead of the manual >> ConsumeBracket / ExpectAndConsume dance. Also, please add some test >> cases for your error recovery (missing ')', missing ']', tokens >> between ']' characters). >> >> Since the presence of attributes in these cases is a hard error, we >> could afford to pass Disambiguate = true to isCXX11AttributeSpecifier. >> Are there any cases in which that would allow us to recover better? >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
