[PATCH] configure.ac: don't use -Wmisleading-indentation compiler option (GCC11)

[Eric Pouech]

in tests, there's a lot of constructs like:

bla1;
    bla2;
todo_wine
    ok(tst, "...");
    bla3;

or

bla1;
    bla2;
    todo_wine
    ok(tst, "...");
    bla3;

GCC11 complains about those (and some more complex) about bla3 not being
properly indented wrt the todo_wine.

I'm not 100% sure about the right fix:
- disable the warning altogether (that's what this patch does), but we
  won't report other *real* indentation errors. To be frank, I haven't
  looked at the hundred of lines of warnings to check if one pops up.
- reformat the code in tests. note that
    todo_wine {
    ok(tst, "...");
    }
  or
    todo_wine ok(tst, "...");
  or
    todo_wine
    ok(tst, "...");
  don't generate warnings. but first one defeats the one liner, and the
  second and third ones don't preserve the line of the test unchanged
  when the todo is resolved

I'd favor disabling the compiler option (it's the less intrusive in the
code and in dev:s habits).

A+
Signed-off-by: Eric Pouech

---
 configure.ac |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure.ac b/configure.ac
index a019b4ac2d6..6cd3e915aae 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1002,6 +1002,7 @@ then
                                   CFLAGS="$CFLAGS $llvm_cflags"])
         fi
         WINE_TRY_CROSSCFLAGS([-fno-strict-aliasing])
+        WINE_TRY_CROSSCFLAGS([-Wno-misleading-indentation])
         dnl clang needs to be told to fail on unknown options
         WINE_TRY_CROSSCFLAGS([-Werror=unknown-warning-option],[CFLAGS="$CFLAGS -Werror=unknown-warning-option"])
         WINE_TRY_CROSSCFLAGS([-Werror=ignored-optimization-argument],[CFLAGS="$CFLAGS -Werror=ignored-optimization-argument"])
@@ -1992,6 +1993,7 @@ then
   WINE_TRY_CFLAGS([-Wunused-but-set-parameter])
   WINE_TRY_CFLAGS([-Wvla])
   WINE_TRY_CFLAGS([-Wwrite-strings])
+  WINE_TRY_CFLAGS([-Wno-misleading-indentation])

if test -z "$CROSSTARGET"
   then

[Zebediah Figura]

On 9/27/21 12:27 PM, Eric Pouech wrote:
> I'm not 100% sure about the right fix:
> - disable the warning altogether (that's what this patch does), but we
>  won't report other *real* indentation errors. To be frank, I haven't
>  looked at the hundred of lines of warnings to check if one pops up.
> - reformat the code in tests. note that
>    todo_wine {
>  ok(tst, "...");
>    }
>  or
>    todo_wine ok(tst, "...");
>  or
>    todo_wine
>  ok(tst, "...");
>  don't generate warnings. but first one defeats the one liner, and the
>  second and third ones don't preserve the line of the test unchanged
>  when the todo is resolved

Personally I think that's a good thing; that allows you to use git-blame 
to find out which commit fixed an ok() message, if any.

[Rémi Bernon]

On 9/27/21 7:30 PM, Zebediah Figura wrote:
> On 9/27/21 12:27 PM, Eric Pouech wrote:
> > I'm not 100% sure about the right fix:
> > - disable the warning altogether (that's what this patch does), but we
> >
> > won't report other *real* indentation errors. To be frank, I haven't
> >    looked at the hundred of lines of warnings to check if one pops up.
> > - reformat the code in tests. note that
> >
> > todo_wine {
> >     ok(tst, "...");
> >      }
> >    or
> >      todo_wine ok(tst, "...");
> >    or
> >      todo_wine
> >     ok(tst, "...");
> >    don't generate warnings. but first one defeats the one liner, and the
> >    second and third ones don't preserve the line of the test unchanged
> >    when the todo is resolved
>
> Personally I think that's a good thing; that allows you to use git-blame 
> to find out which commit fixed an ok() message, if any.

It doesn't warns on cases like:

todo_wine
   ok(...)

Only when the todo_wine is unindented from the next line, and the second 
next line is on the same level, like:

todo_wine
     ok(...)
     ok(...)

Cheers,

[Eric Pouech]

after removing the false positives of GCC11 misleading indentation 
warning, a couple of what looks like true positives:

*dinput8/tests/hid.c:4122 (and same construct line 4480 also)*

<<< current code >>>

if (i == 0 || i == 3) todo_wine_if( i == 0 )
        ok( res == WAIT_TIMEOUT, "WaitForSingleObject succeeded\n" );
        else ok( res == WAIT_OBJECT_0, "WaitForSingleObject failed\n" ); 
ResetEvent( event );

the last ResetEvent( event ) call looks badly placed. could someone 
knowledgeable with the test look into it?

*
**wininet/internet.c:2878*
         if(!lpwhh) {
             SetLastError(ERROR_INTERNET_INCORRECT_HANDLE_TYPE);
             return FALSE;
         } else if(*(ULONG*)lpBuffer & (~(INTERNET_ERROR_MASK_INSERT_CDROM|
                         INTERNET_ERROR_MASK_COMBINED_SEC_CERT|
INTERNET_ERROR_MASK_LOGIN_FAILURE_DISPLAY_ENTITY_BODY))) {
             SetLastError(ERROR_INVALID_PARAMETER);
             ret = FALSE;
         } else if(dwBufferLength != sizeof(ULONG)) {
             SetLastError(ERROR_INTERNET_BAD_OPTION_LENGTH);
             ret = FALSE;
         } else
             TRACE("INTERNET_OPTION_ERROR_MASK: %x\n", *(ULONG*)lpBuffer);
             lpwhh->ErrorMask = *(ULONG*)lpBuffer;

the last assignment (lpwhh->ErrorMask = *(ULONG*)lpBuffer;)
is either badly indented, or misses a block
could someone knowledgeable with the test look into it?

TIA

[Rémi Bernon]

On 9/28/21 2:07 PM, Eric Pouech wrote:
> after removing the false positives of GCC11 misleading indentation 
> warning, a couple of what looks like true positives:
>
> *dinput8/tests/hid.c:4122 (and same construct line 4480 also)*
>
> <<< current code >>>
>
> if (i == 0 || i == 3) todo_wine_if( i == 0 )
>         ok( res == WAIT_TIMEOUT, "WaitForSingleObject succeeded\n" );
>         else ok( res == WAIT_OBJECT_0, "WaitForSingleObject failed\n" ); 
> ResetEvent( event );
>
> the last ResetEvent( event ) call looks badly placed. could someone 
> knowledgeable with the test look into it?

Yes, clang-format messed it up a bit. It should be fixed with the 
patches I sent today.