[PATCH 0/1] MR5148: mshtml: Fix -Wmisleading-indentation warnings.
This warning spams the build. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5148
From: Vijay Kiran Kamuju <infyquest(a)gmail.com> --- dlls/mshtml/mshtml_private.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 9609fb5edcb..153d5b5b912 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -1440,7 +1440,8 @@ static inline char *strndupWtoU(const WCHAR *str, unsigned len) size = len ? WideCharToMultiByte(CP_UTF8, 0, str, len, NULL, 0, NULL, NULL) : 0; ret = malloc(size + 1); if(ret) { - if(len) WideCharToMultiByte(CP_UTF8, 0, str, len, ret, size, NULL, NULL); + if(len) + WideCharToMultiByte(CP_UTF8, 0, str, len, ret, size, NULL, NULL); ret[size] = '\0'; } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5148
are you sure it fixes the warning? (it doesn't here) what works here is to pass -flarge-source-files to gcc but that generates a ~8% compilation time increase (YMMV), so not sure we want this as a 'all modules' option (we could add it on a per DLL option though) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5148#note_62509
It does not work, we can add -flarge-source-files to extraflags in Makefile.in for mshtml -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5148#note_62510
you can't do it directly in Makefile.in (as this option as been added in GCC11, so we need to support compilers without this option) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5148#note_62511
Another option is to move some of the inline functions to wine internal headers and split the header to smaller ones -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5148#note_62512
FWIW, it's really a GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89549 The main source of mshtml file sizes is mshtml.h, which is a very large generated header. There are some possible improvements in mshtml internals, some of which would be nice to have regardless, but ultimately mshtml.idl is public and will continue to grow. Unfortunately, GCC also ignores `#pragma` here, so it's not easy to have a targeted workaround. Doing that globally is not free. As Eric mentioned, `-flarge-source-files` has compile time penalty. Another option would be to just disable `-Wmisleading-indentation` or hack makedep. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5148#note_62574
On Sun Feb 25 18:22:18 2024 +0000, Jacek Caban wrote:
FWIW, it's really a GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89549 The main source of mshtml file sizes is mshtml.h, which is a very large generated header. There are some possible improvements in mshtml internals, some of which would be nice to have regardless, but ultimately mshtml.idl is public and will continue to grow. Unfortunately, GCC also ignores `#pragma` here, so it's not easy to have a targeted workaround. Doing that globally is not free. As Eric mentioned, `-flarge-source-files` has compile time penalty. Another option would be to just disable `-Wmisleading-indentation` or hack makedep. I meant `-Wno-misleading-indentation`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5148#note_62576
On Sun Feb 25 18:22:17 2024 +0000, Jacek Caban wrote:
I meant `-Wno-misleading-indentation`. I believe, we cannot completely disable -Wno-misleading-indentation as it might help us write readable code.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5148#note_62579
On Sun Feb 25 18:32:47 2024 +0000, Vijay Kiran Kamuju wrote:
I believe, we cannot completely disable -Wno-misleading-indentation as it might help us write readable code. Has -Wmisleading-indentation really helped? As far as I've seen it's only come up with "false positives", i.e. not real bugs, and gotten in our way.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5148#note_62581
On Sun Feb 25 19:08:27 2024 +0000, Zebediah Figura wrote:
Has -Wmisleading-indentation really helped? As far as I've seen it's only come up with "false positives", i.e. not real bugs, and gotten in our way. I see that in early 2022, there were a lot of cleanup patches related to misleading indentation. While a lot of it was just indentation of todo_wine, I did a see a few that actually improved that code, and possibly caught a few bugs.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5148#note_62725
I see that in early 2022, there were a lot of cleanup patches related to misleading indentation. While a lot of it was just indentation of todo_wine, I did a see a few that actually improved that code, and possibly caught a few bugs.
Which ones are you referring to? Through a brief search I found *one* commit that actually changed behaviour, namely ac1628c9d6. Maybe two others I found were even a visual improvement (08306780d, a51e20f5e0), though those weren't from 2022. The rest were mostly todo_wine, and the ones that weren't were really debatably even an improvement. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5148#note_62759
On Mon Feb 26 22:16:33 2024 +0000, Zebediah Figura wrote: > > I see that in early 2022, there were a lot of cleanup patches related > to misleading indentation. While a lot of it was just indentation of > todo_wine, I did a see a few that actually improved that code, and > possibly caught a few bugs. > Which ones are you referring to? > Through a brief search I found *one* commit that actually changed > behaviour, namely ac1628c9d6. Maybe two others I found were even a > visual improvement (08306780d, a51e20f5e0), though those weren't from > 2022. The rest were mostly todo_wine, and the ones that weren't were > really debatably even an improvement. I hadn't looked through them all, but I think they were pretty much the ones you found. I think a44794af may have been an improvement too. Missed that some of those were older (though still probably caught by the flag). I guess I don't really mind removing the flag, but I would say it has had _some_ value (if rather small), and at least I don't think fixing the 'false positives' made the code any worse. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5148#note_62763
This merge request was closed by Vijay Kiran Kamuju. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5148
participants (6)
-
eric pouech (＠epo) -
Jacek Caban (＠jacek)
-
Jeffrey Smith (＠whydoubt)
-
Vijay Kiran Kamuju
-
Vijay Kiran Kamuju (＠infyquest)
-
Zebediah Figura (＠zfigura)