Re: RFC: [GUPC] UPC-related front-end changes

["Joseph S. Myers" <joseph at codesourcery dot com>]

On Wed, 7 Jul 2010, Gary Funck wrote:

> The UPC-related front-end "diff's" are attached
> for review.  All feedback and suggestions

This patch does not seem to be in a state in which it is ready for 
detailed technical review of whether it implements correct semantics.

* Before posting a patch for review, clean up FIXMEs and TODOs.

* There are a large number of copyright and license notices in an 
extremely obsolete form, referencing "GNU CC".  Make sure to use the 
current standard forms of all such notices.

* Review code for correct conformance to coding standards.  Comments 
before all functions that explain the function semantics, all parameters 
and the return value.  Spaces before parentheses where appropriate.  
Diagnostics following GNU standards including no closing "." and starting 
with lowercase.

* Do not try to build up diagnostics with sprintf; this is unfriendly to 
i18n.  Put appropriate format strings directly in the calls to diagnostic 
functions.  Avoid direct use of IDENTIFIER_POINTER in diagnostics as that 
is also unfriendly to i18n; use formats such as %qD instead.

* It is not acceptable for files in c-family/ to include c-tree.h, because 
c-tree.h describes facilities only available in the C and ObjC front ends 
and not available for C++.  Do not revert recent modularity improvements 
such as this.

* It appears you have a large number of #ifdef conditionals with no 
obvious reason for the macros being conditionally defined or not defined.  
The use of conditional compilation like this is deprecated for new code.  
If something may vary from target to target, use target hooks, not macros, 
and document them in tm.texi (if they are in fact documented there in 
something outside this patch, perhaps you need to post that other patch).  
Conditions using "if" are strongly preferred to those using "#if" whenever 
possible.

* In general, since this code has evidently been around for a very long 
time given such things as use of "GNU CC", it needs careful review for 
whether it accords with current GCC coding practices for new code and 
takes account of cleanups done in the past decade or so, rather than with 
the practices of a decade ago.

* Make sure the diffs do not make unrelated changes to code not otherwise 
being modified.  For example, the

@@ -5387,7 +5471,7 @@ grokdeclarator (const struct c_declarato

diff hunk appears just to be changing the indentation of a comment, and 
while that looks like a correct change it's got nothing to do with UPC, so 
should go in trunk separately.

-- 
Joseph S. Myers
joseph@codesourcery.com