Fixing semantic diagnostics that embed english words

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Fixing semantic diagnostics that embed english words

Nicola Gigante
Hello,

I'm sending a patch that fixes some diagnostic messages that used to embed english words as arguments.
Previously, those string were flying around the various Sema*.cpp files as a const char *Flavor argument passed
to the DiagnoseAssignmentResult() method and a couple of others.

I've replaced those strings with values from an AssignmentAction enumeration declared in Sema.h around line 830. Is it ok to do things like that? The constants' name of the enumeration are very similar to the strings that they represents.

In the meantime, I've found that a similar Flavor argument was passed to BuildCXXDerivedToBaseExpr() method but that it wasn't used anywhere so I've removed it, is it ok?

I've run the tests and everything seems ok.

I hope this is useful, bye bye,

Nicola


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

diag_assignment_messages.patch (31K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Fixing semantic diagnostics that embed english words

Douglas Gregor

On Dec 12, 2009, at 11:38 AM, Nicola Gigante wrote:

> Hello,
>
> I'm sending a patch that fixes some diagnostic messages that used to embed english words as arguments.
> Previously, those string were flying around the various Sema*.cpp files as a const char *Flavor argument passed
> to the DiagnoseAssignmentResult() method and a couple of others.
>
> I've replaced those strings with values from an AssignmentAction enumeration declared in Sema.h around line 830. Is it ok to do things like that? The constants' name of the enumeration are very similar to the strings that they represents.

Yes, this is very helpful! We generally prefix enumerator names with something resembling the enumeration name, so that we're less likely to have conflicts. For example, in the names here:

+  enum AssignmentAction {
+    Assigning,
+    Copying,
+    Passing,
+    Returning,
+    Converting,
+    Initializing,
+    Sending,
+    Casting
+  };

we would prefer to use the names AA_Assigning, AA_Copying, AA_Passing, etc.

> In the meantime, I've found that a similar Flavor argument was passed to BuildCXXDerivedToBaseExpr() method but that it wasn't used anywhere so I've removed it, is it ok?

Yes, thanks!

Thank you for fixing this; if you could perform the rename mentioned above, I'd be happy to commit your patch.

        - Doug
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Fixing semantic diagnostics that embed english words

Nicola Gigante

Il giorno 13/dic/2009, alle ore 21.37, Douglas Gregor ha scritto:

>
> Thank you for fixing this; if you could perform the rename mentioned above, I'd be happy to commit your patch.
>

Here it is :)

> - Doug

Nicola


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

diag_assignment_messages.patch (31K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

clang-cc no more

jahanian
In reply to this post by Douglas Gregor
Hi All,

Make sure to remove clang-cc from you path when testing. Not all tests  
have been converted to
use 'clang -cc1...'.  I am modifying all objc test now.

- Fariborz

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Fixing semantic diagnostics that embed english words

Douglas Gregor
In reply to this post by Nicola Gigante

On Dec 13, 2009, at 1:43 PM, Nicola Gigante wrote:

>
> Il giorno 13/dic/2009, alle ore 21.37, Douglas Gregor ha scritto:
>
>>
>> Thank you for fixing this; if you could perform the rename mentioned above, I'd be happy to commit your patch.
>>
>
> Here it is :)


Great, thanks! Committed here:

        http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20091214/025470.html

        - Doug
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Loading...