cmake: enable -Wall for MSVC when PICKY_COMPILER=ON

[vszakats]

Enable it for _MSC_VER <= 1943.

Omit it for not yet CI-tested MSVC versions, to avoid hitting unfixed
warnings emitted by future toolchain releases. It means we need
to explicitly opt-in to newer MSVC versions while fixing any new issues.

The newly enabled warnings did not reveal new issues. It hints that we
catch those with clang/gcc. Yet, these warnings may be useful for local
development done with MSVC.

Also:

• disable and document warnings that don't seem useful, unactionable,
  or unfixable.
• disable and document warnings found in Windows SDK headers.
• tidy up a few comments, also to avoid
  -Wdocumentation-unknown-command, part of llvm/clang -Wall.

  lib\dynhds.h(159,29): error : unknown command tag name [-Werror,-Wdocumentation-unknown-command]
  lib\ftp.c(337,15): error : unknown command tag name [-Werror,-Wdocumentation-unknown-command]
  

  (This patch did not end up enabling -Wall for clang-cl.)

[MarcelRaad]

In my experience, keeping code /Wall clean only makes sense when supporting few compiler versions. Pretty much every new MSVC update breaks it, so I wouldn't use it for a public library. At least for C++, it might be less breaking for C.

which we may need to fix or opt-out from on upgrade

We can opt out our own CI, but we cannot do that for all our users. Might a CI job setting these flags manually be an alternative?

[vszakats]

In my experience, keeping code /Wall clean only makes sense when supporting few compiler versions. Pretty much every new MSVC update breaks it, so I wouldn't use it for a public library. At least for C++, it might be less breaking for C.

which we may need to fix or opt-out from on upgrade

It's a downside, I agree.

We can opt out our own CI, but we cannot do that for all our users. Might a CI job setting these flags manually be an alternative?

It is, or, it's also an option to limit it to the latest CI-tested MSVC version
(VS2022) to avoid suprises on upgrade (or compiling this curl version in
the future with a then current MSVC.)

I'd prefer the latter, because this may rather help someone contributing
for curl using MSVC. In CI, it seems we already hit and fix these warnings
by finding them through gcc/clang.

Though the effect may be limited or perhaps overkill and nobody asked
for this. So, a 3rd option is to ditch this PR entirely.

[MarcelRaad]

It is, or, it's also an option to limit it to the latest CI-tested MSVC version
(VS2022) to avoid suprises on upgrade (or compiling this curl version in
the future with a then current MSVC.)

Limiting this to a certain MSVC version makes sense to me too! Only keep in mind that new warnings come in minor versions, so we would need to use e.g. Visual Studio 2022 update 14 instead of just Visual Studio 2022.

[vszakats]

Pushed a commit that draws the limit at MSVC_VERSION <= 1943.
Where 1943 is Visual Studio 2022 version 17.13 ¹, the version running
in GHA with windows 2022 runners.

What do you think?

Footnotes

1. https://learn.microsoft.com/cpp/overview/compiler-versions?view=msvc-170 ↩

[bagder]

I would nominate this as one of the most stupid compiler warnings in recent days.

Does it perhaps also go away if the /** comment start is converted into a plain /* ?

[vszakats]

This wasn't the only one, it was catching email addresses in the copyright
header too, but curiously only a couple. It's not an MSVC warning, but an
llvm/clang one: -Wdocumentation-unknown-command.

The only reason I bothered fixing this one because other comments around
here prefer the CR/LF terminology. (Though the casing is still inconsistent.)

Yes, changing /** to /* also silences it. Weird.

lib/bufref.h:10:42: error: unknown command tag name [-Werror,-Wdocumentation-unknown-command]
 * Copyright (C) Daniel Stenberg, <daniel@haxx.se>, et al.
                                         ^~~~~
In file included from lib/altsvc.c:32:
In file included from lib/vssh/../urldata.h:193:
lib/smb.h:10:36: error: unknown command tag name [-Werror,-Wdocumentation-unknown-command]
 * Copyright (C) Bill Nagel <wnagel@tycoint.com>, Exacq Technologies
                                   ^~~~~~~~
lib/ftp.c:337:15: error: unknown command tag name [-Werror,-Wdocumentation-unknown-command]
 * ending in '\r\n' and we prefer just '\n'.                  
              ^~                                              
lib/ftp.c:337:17: error: unknown command tag name [-Werror,-Wdocumentation-unknown-command]
 * ending in '\r\n' and we prefer just '\n'.                  
                ^~                                            
4 errors generated.                                                           

edit: silenced the ftp.c one too by dropping the *** decoration used
in that particular comment.