[cfe-commits] [Patch] Fix for PR2709

[Douglas Gregor]

On Dec 5, 2011, at 3:35 AM, Erik Verbruggen wrote:

> On 2-12-11 16:10, Douglas Gregor wrote:
>> 
>> On Dec 2, 2011, at 5:03 AM, Erik Verbruggen wrote:
>> 
>>> On 1-12-11 17:19, Douglas Gregor wrote:
>>>> I'd prefer this solution, which just asks Sema whether we're already inside an ObjC container. Then the parser will handle recovery for what is essentially a parsing issue, calling Sema's ActOnAtEnd and producing the error/Fix-It.
>>> 
>>> Nearly done, just one thing left. Considering this snippet:
>>> 
>>> @interface A
>>> -(void) im0;
>>> 
>>> @implementation A
>>> @end
>>> 
>>> The fix-it is clearly an "insert @end before @implementation". But should the error be attached to the @interface or the @implementation? I have a slight preference for the @interface: if the last @end was missing, there would be 2 fix-its on the @implementation (one for the interface, and the other because there is just no token after that line).
>> 
>> 
>> The error should go on the @implementation, with a note pointing to the @interface.
>> 
>> 	- Doug
> 
> Attached is the reworked patch.

index 7d8f213..2effe64 100644
--- a/include/clang/Basic/DiagnosticParseKinds.td
+++ b/include/clang/Basic/DiagnosticParseKinds.td
@@ -313,7 +313,8 @@ def err_expected_minus_or_plus : Error<
   "method type specifier must start with '-' or '+'">;
 def err_objc_no_attributes_on_category : Error<
   "attributes may not be specified on a category">;
-def err_objc_missing_end : Error<"missing @end">;
+def err_objc_missing_end : Error<"missing '@end'">;
+def note_objc_container_start : Note<"Objective-C container started here">;

We have never used the term "container" before in a diagnostic (even though we have it in Clang), and I don't think we should. Objective-C programmers don't think of @interface/@implementation/@protocol/etc. as "containers", and would be confused by this. How about specializing this based on the kind of declaration? For example:

+def note_objc_container_start : Note<"%select{class|protocol|category|class extension|implementation|category implementation}0 started here">;

Actions.isObjCContainerContext() could return an enum, rather than a bool, specifying which of these options to use.

Sorry I missed "container" bit earlier. There really is no good way to say "Objective-C class or protocol-like thing" that ObjC programmers would generally recognize.

Everything else looks great! Feel free to commit with the requested change.

	- Doug