[release/3.1] Fix build failures with clang 13 rc1

[omajid]

Clang 13 now produces an error in json/casablanca because a function
argument is not used:

In file included from /core-setup/src/corehost/cli/deps_format.cpp:6:
In file included from /core-setup/src/corehost/cli/deps_format.h:14:
In file included from /core-setup/src/corehost/cli/fxr/../json/casablanca/include/cpprest/json.h:36:
In file included from /core-setup/src/corehost/cli/fxr/../json/casablanca/include/cpprest/details/basic_types.h:41:
/core-setup/src/corehost/cli/fxr/../json/casablanca/include/cpprest/details/SafeInt3.hpp:1317:33: error: parameter 'b' set but not used [-Werror,-Wunused-but-set-parameter]
    static void CastThrow( bool b, T& t ) SAFEINT_CPP_THROW
                                ^
1 error generated.
gmake[2]: *** [cli/fxr/CMakeFiles/hostfxr.dir/build.make:76: cli/fxr/CMakeFiles/hostfxr.dir/__/deps_format.cpp.o] Error 1

Cast the argument to void to avoid this warning/error.

[Anipik]

cc @NikolaMilosavljevic can you review this change

[omajid]

Ping. Anyone want to review this?

[Anipik]

cc @jeffschwMSFT @aik-jahoda

[jeffschwMSFT]

Adding @janvorli and @mangod9

[mangod9]

The change itself looks innocuous, but I am not sure about whether updating to clang 13 is required for rc1?

[janvorli]

@mangod9 the "rc1" refers to clang 13 rc1, not .NET rc1.

[safern]

Is this change still needed?

[omajid]

It would be nice to include this to support building against clang 13. That said, this change is now known to be incomplete; we will need the changes from dotnet/runtime#63314 as well.

[safern]

I see. Should we then close this PR until it can be worked on to get it to a complete state?

[omajid]

I converetd the PR to draft, let me know if it's still better to close it until I get a chance to re-visit it.

[safern]

That's good. Thanks, it just helps to have only open PRs on these repos that are targeting the next servicing release.

[omajid]

Uh.. I opened #9197 yesterday which fixes the same issue differently. Any thoughts which approach is better?

[omajid]

Any thought about this?

[NikolaMilosavljevic]

@omajid where do you see this error? I presume this is a local dev build, as we do not get this error in official builds.

[NikolaMilosavljevic]

cc @vitek-karas

[omajid]

@NikolaMilosavljevic I am one of the maintainer of the .NET Core 3.1 package in Fedora. Building core-setup fails on Fedora 36 (which uses clang 14) because core-setup enables -Werror and then has code that produces warnings.

We should either fix the warnings, or disable -Werror.

The issue affects other Linux distributions as well, including Alpine: https://gitlab.alpinelinux.org/alpine/aports/-/tree/master/testing/dotnet31-build (see core-setup_clang13-no-werror.patch)

[NikolaMilosavljevic]

@vitek-karas this is host code - can you take a look?

[vitek-karas]

Actually @janvorli is a much better for this type of change.

One question - the linked change in the runtime dotnet/runtime#63314 looks different - is there a reason for this difference?

[omajid]

@vitek-karas There's a few individual fixes needed to get all of .NET Core 3.1 building under clang 13.

This fix, which updates the SafeInt3.hpp file, doesn't apply in runtime. As far as I can tell, there's no SafeInt3.hpp in runtime at all.

The rest of the fixes in dotnet/runtime#63314 take care of the changes that would be needed in core-setup coreclr and corefx for 3.1.

[omajid]

Any updates on this? Is this okay to merge?

[jeffschwMSFT]

we need to take for approval. I will mark it as such.

[jeffschwMSFT]

Approved. We will take for consideration in 3.1.x

[omajid]

Thanks!