This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
2010/4/16 Joseph S. Myers <joseph@codesourcery.com>: > On Fri, 16 Apr 2010, Kai Tietz wrote: > >> 2010/4/15 Joseph S. Myers <joseph@codesourcery.com>: >> > You don't appear to have addressed my previous review comments in this >> > patch version (for example, the point that __int128 should be handled like >> > int, void, _Bool etc., getting "two or more data types in declaration >> > specifiers" for use with those specifiers instead of having explicit >> > checks for all of them). ?Furthermore, there are changes to existing code >> > (patch hunks that just change whitespace, for example) that appear >> > irrelevant to the addition of __int128. >> > >> > -- >> > Joseph S. Myers >> > joseph@codesourcery.com >> > >> >> Hmm, I just re-read my patch I sent and what white-space changes you >> are talking about? I don't see in this patch pure white-space changes. > > See the @@ -8725,6 +8738,7 @@ hunk for example, inserting a blank line, or > @@ -9024,7 +9105,7 @@, changing indentation. ?I refer to > <http://gcc.gnu.org/ml/gcc-patches/2010-04/msg00354/int128doc.diff>. I see, but I don't see an new-line in this hunk. I see that leading spaces got converted to tabs. And I admit this is an unrelated hunk for this patch. At another place I added an new line to for read-ability, but I revert it in my updated patch. >> Also I modified in this patch that __int128 is handled like void, >> _Bool, int, float, etc. I don't see you point. Could it be that you've > > If it's handled like them, you wouldn't need special diagnostics such as > "both %<__int128%> and %<char%> in declaration specifiers" because the > generic diagnostic I mentioned would have caught this earlier. ?And the > check in the RID_LONG case would go with all the other checks for > particular keywords such as _Bool, rather than before the "long long long" > check in your present patch; likewise for RID_SHORT and probably other > cases. I see and indeed it is useless code as condition will never hit as it is checked already before. >?The falling through from cts_int128 to cts_int is also suspicious > since almost none of the code is actually relevant to both cases. Yeah, this fall-through to int-case wasn't wrong in terms of logic but piggy as the check for int128 in this case is misleading. The check in short-case is superfluous. New patch tested for i686-pc-linux-gnu, i686-pc-cygwin, i686-pc-mingw32, x86_64-pc-mingw32, and x86_64-pc-linux-gnu. Ok for apply? Thanks, Kai -- | (\_/) This is Bunny. Copy and paste | (='.'=) Bunny into your signature to help | (")_(") him gain world domination
Attachment:
int128doc.diff
Description: Binary data
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |