llvm dev - Dec 2007 - [LLVMdev] fix warning with newer g++ compilers

Newer g++ compilers can emit:

/Volumes/mrs5/net/llvm/llvm/llvm/lib/AsmParser/LLLexer.cpp: In member  
function 'int llvm::LLLexer::LexAt()':
/Volumes/mrs5/net/llvm/llvm/llvm/lib/AsmParser/LLLexer.cpp:287:  
warning: suggest a space before ';' or explicit braces around empty  
body in 'for' statement
/Volumes/mrs5/net/llvm/llvm/llvm/lib/AsmParser/LLLexer.cpp: In member  
function 'int llvm::LLLexer::LexPercent()':
/Volumes/mrs5/net/llvm/llvm/llvm/lib/AsmParser/LLLexer.cpp:338:  
warning: suggest a space before ';' or explicit braces around empty  
body in 'for' statement
/Volumes/mrs5/net/llvm/llvm/llvm/lib/AsmParser/LLLexer.cpp: In member  
function 'int llvm::LLLexer::LexDigitOrNegative()':
/Volumes/mrs5/net/llvm/llvm/llvm/lib/AsmParser/LLLexer.cpp:720:  
warning: suggest a space before ';' or explicit braces around empty  
body in 'for' statement
/Volumes/mrs5/net/llvm/llvm/llvm/lib/AsmParser/LLLexer.cpp: In member  
function 'int llvm::LLLexer::LexPositive()':
/Volumes/mrs5/net/llvm/llvm/llvm/lib/AsmParser/LLLexer.cpp:789:  
warning: suggest a space before ';' or explicit braces around empty  
body in 'for' statement

The usual style is to put a space before the ;, or put the ; on a new  
line to visually separate the ; from the for statement, so that the  
follow like doesn't look like it goes with the for.

Thoughts?


Index: lib/AsmParser/LLLexer.cpp
==================================================================---
lib/AsmParser/LLLexer.cpp	(revision 45058)
+++ lib/AsmParser/LLLexer.cpp	(working copy)
@@ -284,7 +284,7 @@

    // Handle GlobalVarID: @[0-9]+
    if (isdigit(CurPtr[0])) {
-    for (++CurPtr; isdigit(CurPtr[0]); ++CurPtr);
+    for (++CurPtr; isdigit(CurPtr[0]); ++CurPtr) ;

      uint64_t Val = atoull(TokStart+1, CurPtr);
      if ((unsigned)Val != Val)
@@ -335,7 +335,7 @@

    // Handle LocalVarID: %[0-9]+
    if (isdigit(CurPtr[0])) {
-    for (++CurPtr; isdigit(CurPtr[0]); ++CurPtr);
+    for (++CurPtr; isdigit(CurPtr[0]); ++CurPtr) ;

      uint64_t Val = atoull(TokStart+1, CurPtr);
      if ((unsigned)Val != Val)
@@ -717,7 +717,7 @@
    // At this point, it is either a label, int or fp constant.

    // Skip digits, we have at least one.
-  for (; isdigit(CurPtr[0]); ++CurPtr);
+  for (; isdigit(CurPtr[0]); ++CurPtr) ;

    // Check to see if this really is a label afterall, e.g. "-1:".
    if (isLabelChar(CurPtr[0]) || CurPtr[0] == ':') {
@@ -786,7 +786,7 @@
      return CurPtr[-1];

    // Skip digits.
-  for (++CurPtr; isdigit(CurPtr[0]); ++CurPtr);
+  for (++CurPtr; isdigit(CurPtr[0]); ++CurPtr) ;

    // At this point, we need a '.'.
    if (CurPtr[0] != '.') {

Hi,
> -    for (++CurPtr; isdigit(CurPtr[0]); ++CurPtr);
> +    for (++CurPtr; isdigit(CurPtr[0]); ++CurPtr) ;
personally I like

	for (++CurPtr; isdigit(CurPtr[0]); ++CurPtr) {}

better since it is more explicit.

Ciao,

Duncan.

Hi Mike,
> The usual style is to put a space before the ;, or put the ; on a new
> line to visually separate the ; from the for statement, so that the
> follow like doesn't look like it goes with the for.
The "usual" styles I'm used to seeing, dating back almost 20
years, are
the semi-colon up tight to the closing parenthesis, as LLLexer.cpp has,
or

    for (fee(); fie(); foe()) {
        /* Do nothing. */
    }

However, I see indent(1) has a -ss option to insert a single space
before the semi-colon so the convention must have been around a while.

Personally, I think the lack of a following indented statement or block
makes it clear to look for the semi-colon if it hasn't already been
spotted and the warning's overkill.

Cheers,


Ralph.

On Dec 14, 2007, at 11:10 PM, Mike Stump wrote:
> Newer g++ compilers can emit:
>
> /Volumes/mrs5/net/llvm/llvm/llvm/lib/AsmParser/LLLexer.cpp:789:
> warning: suggest a space before ';' or explicit braces around empty
> body in 'for' statement
>
> The usual style is to put a space before the ;, or put the ; on a new
> line to visually separate the ; from the for statement, so that the
> follow like doesn't look like it goes with the for.
Hey Mike,

Thanks for the patch.  It doesn't apply because your mailer included  
it inline.  If you're using the apple mailer, putting a .patch suffix  
on it (instead of .txt) might help.

w.r.t the debate, I'd prefer:

  for (++CurPtr; isdigit(CurPtr[0]); ++CurPtr)
    /*empty*/;

but I really don't care that much :)

-Chris

On Dec 15, 2007, at 2:13 AM, Duncan Sands wrote:
> Hi,
>
>> -    for (++CurPtr; isdigit(CurPtr[0]); ++CurPtr);
>> +    for (++CurPtr; isdigit(CurPtr[0]); ++CurPtr) ;
>
> personally I like
>
> 	for (++CurPtr; isdigit(CurPtr[0]); ++CurPtr) {}
>
> better since it is more explicit.
I think the bikeshed should be orange:

	for (++CurPtr; isdigit(CurPtr[0]); ++CurPtr)
              ;