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

[Richard Smith]

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
>>>>
>>>>
>>>
>>
>