[cfe-commits] [PATCH] Add Fixit for C++11 attributes that appertain to class specifiers.

[Michael Han]

Hi Richard,

Updated patch attached, please take a look. Sorry for the long delay of
reworking this patch.

Michael

On Tue, Dec 18, 2012 at 12:49 AM, Richard Smith <richard at metafoo.co.uk>wrote:

> Sorry for the delay!
>
> +void Parser::CutAndPasteCode(SourceRange Range,
> +                             SourceLocation InsertLocation,
> +                             unsigned int DiagID) {
> +  SourceManager &SM = PP.getSourceManager();
> +  const char *Start = SM.getCharacterData(Range.getBegin());
> +  const char *End = SM.getCharacterData(Range.getEnd());
> +  SmallString<64> Code(Start, End);
>
> This isn't going to work if the ends of the range point into different
> FileIDs. In any case, it's unnecessary; use
> FixItHint::CreateInsertionFromRange instead.
>
> +    if (Attributes.Range.isValid()) {
> +      CutAndPasteCode(Attributes.Range, AttrLoc,
> +                      diag::err_attributes_not_allowed);
> +      Attributes.clear();
> +    }
>
> Clearing the attributes here doesn't look right: after issuing a
> fix-it for an error, you must recover as if the fix-it were applied
> (so the attributes must actually be applied to the class).
>
> +    // any attributes after class-name, we try a fixit to move
> +    // them to right place.
>
> ... to *the* right place.
>
> +    // Parse any C++11 attributes after 'final'keyword
>
> Missing space after 'final'. Missing full stop.
>
> On Mon, Dec 17, 2012 at 9:56 PM, Michael Han <fragmentshaders at gmail.com>
> wrote:
> > Reviving an old patch. Is this patch OK to commit?
> >
> > Cheers
> > Michael
> >
> >
> > On Thu, Dec 6, 2012 at 9:39 AM, Michael Han <fragmentshaders at gmail.com>
> > wrote:
> >>
> >> Hi Richard,
> >>
> >> Do you also have a chance to look at this patch? Please let me know if
> >> it's OK to commit it. Thanks!
> >>
> >> Michael
> >>
> >> On Mon, Dec 3, 2012 at 11:00 AM, Michael Han <fragmentshaders at gmail.com
> >
> >> wrote:
> >>>
> >>> I see. Thanks for the explanation!
> >>>
> >>> Michael
> >>>
> >>> On Mon, Dec 3, 2012 at 9:38 AM, Jordan Rose <jordan_rose at apple.com>
> >>> wrote:
> >>>>
> >>>> Some of them are primarily testing the caret fixits, but you're right
> >>>> that the parseable fixits don't actually prevent you from testing the
> caret
> >>>> fixits. On the other hand, until recently FileCheck didn't have a way
> to
> >>>> specify relative line numbers, meaning that adding a new line at the
> top of
> >>>> the file would change all of the parseable fixits (unless matched with
> >>>> wildcards).
> >>>>
> >>>> Going forward we should probably endeavour to test the parseable
> fixits
> >>>> for every test where we're not just testing caret fixit emission.
> >>>>
> >>>> Jordan
> >>>>
> >>>>
> >>>> On Dec 1, 2012, at 21:05 , Michael Han <fragmentshaders at gmail.com>
> >>>> wrote:
> >>>>
> >>>> Hi Dmitri,
> >>>>
> >>>> Thanks! Attach updated patch.
> >>>>
> >>>> I noticed that not all test cases under clang/test/FixIt use
> >>>> "-fdiagnostics-parseable-fixits". Is there a reason why not all FixIt
> test
> >>>> cases use this option?
> >>>>
> >>>> Cheers
> >>>> Michael
> >>>>
> >>>> On Sat, Dec 1, 2012 at 5:21 AM, Dmitri Gribenko <gribozavr at gmail.com>
> >>>> wrote:
> >>>>>
> >>>>> On Sat, Dec 1, 2012 at 6:30 AM, Michael Han <
> fragmentshaders at gmail.com>
> >>>>> wrote:
> >>>>> > This patch adds two FixIts to parser when parsing C++11 attributes
> >>>>> > appertain
> >>>>> > to class specifier. It is a following up patch of a previous patch
> >>>>> > [1].
> >>>>> >
> >>>>> > Please review, thanks!
> >>>>>
> >>>>> Hello Michael,
> >>>>>
> >>>>> FixIts can be tested with -fdiagnostics-parseable-fixits (search the
> >>>>> testsuite for this option to find examples).
> >>>>>
> >>>>> Dmitri
> >>>>>
> >>>>> --
> >>>>> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> >>>>> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
> >>>>
> >>>>
> >>>> <fixit.patch>_______________________________________________
> >>>> cfe-commits mailing list
> >>>> cfe-commits at cs.uiuc.edu
> >>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >>>>
> >>>>
> >>>
> >>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130104/db2f7721/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fixit.patch
Type: application/octet-stream
Size: 7212 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130104/db2f7721/attachment.obj>