[PATCH] configure.ac: don't use -Wmisleading-indentation compiler option (GCC11)
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 <eric.pouech(a)gmail.com> --- 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
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.
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, -- Rémi Bernon <rbernon(a)codeweavers.com>
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
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. -- Rémi Bernon <rbernon(a)codeweavers.com>
participants (4)
-
Eric Pouech -
Eric Pouech -
Rémi Bernon -
Zebediah Figura