Compile warning free

[joeduffy]

Lots of warnings right now.

Mostly due to us having started on MSVC, rather than Clang.

We need to clean this up and lock it in with the right switches (WX, Werror, etc).

[pgavlin]

Unfortunately, most of these warning come from the CoreCLR headers, which as you note require MS extensions in order to compile without warnings. In order to get rid of these warnings, we'll have to fix the CoreCLR headers (or find some way--perhaps a pragma, if one exists--to enable MS extensions only for those headers).

[russellhadley]

This has been an outstanding issue for a bit. @mmitche had to work around some of this along the way for xplat. It'd be nice if the long range plan was to fix this up but we're going to need to do this work mostly in CoreCLR. @jkotas, is long range plan to fix the Cor* headers so that they're cross platform warning free?

[jkotas]

@janvorli has done pass over the LLVM C++ warnings in CoreCLR, fixing the ones that looked important to fix, disabling rest as necessary. We do not have plans to do more.

Feel free to submit PRs against CoreCLR if there are things you care about getting fixed.

[pgavlin]

FWIW, it would also be nice if the cor* headers were usable without the declarations in the LLILC PAL.

[janvorli]

Actually, there are still some that I've left for the jitter team to clean up based on my discussion with @cmckinsey (#598) that are still pending.

[janvorli]

@jkotas There are some more left that I want to fix, I have just postponed it due to other more pressing issues. Here is an overview of all the remaining disabled warnings with some description added:

-Wno-unused-private-field

There are private fields in classes that are not being used. There are two kinds of these. One are members that are really obsolete and not used anymore. The other is caused by #ifdefs that for some combinations of defines cause the member to not to be used.

-Wno-dangling-else

There are conditions like

if () {...} else command;

These are mostly generated by the following macro:

#define return if (0 && __ReturnOK::safe_to_return()) { } else return

-Wno-implicit-exception-spec-mismatch

Function previously declared with an explicit exception specification redeclared with an implicit exception specification - all on operator delete

-Wno-overloaded-virtual

A derived class defines a virtual method with the same name as its base class, but different set of parameters.

-Wno-unused-variable

There are locals that are not used.

-Wno-switch

Enum value is not handled in a switch (and there is no default clause). Remaining cases are in JIT only

-Wno-microsoft

Explicit constructor calls are not supported by clang (this->ClassName::ClassName())

-Wno-tautological-compare

This warning is caused by comparing 'this' to NULL

-Wno-constant-logical-operand

There are constants of type BOOL used in a condition. But BOOL is defined as int and so the compiler thinks that there is a mistake.

-Wno-unknown-warning-option

Some of the warning disabling options exist only on clang 3.6 so clang 3.5 complained it doesn't know them.

-Wno-invalid-offsetof

__builtin_offsetof(type, field) is applied to a non-standard-layout type

-Wno-delete-non-virtual-dtor

Class with pure virtual methods has non-virtual destructor

-Wno-incompatible-ms-struct

The warning indicates that an attribute __attribute__((__ms_struct__)) was applied to a struct or a class that has virtual members or a base class. In that case, clang may not generate the same object layout as MSVC.

[russellhadley]

@janvorli is there an issue for this in CoreCLR we should link to? We'd want to do the work there. Another question: Did you expect to be able to fix these on the GitHub CoreCLR side?

[janvorli]

Ah, I got confused, I haven't realized this is not the CoreCLR but the llilc.
We don't have a github issue for the remaining warnings yet, but it is a good opportunity to create one.
Regarding the question about being able to fix them, the following ones should be reasonably fixable:
-Wno-implicit-exception-spec-mismatch
-Wno-switch
-Wno-microsoft
-Wno-constant-logical-operand
-Wno-delete-non-virtual-dtor
-Wno-unused-variable

The following ones are more difficult to fix:
-Wno-overloaded-virtual - it seems it would require renaming the methods or figuring out if there is an attribute to mark those methods
-Wno-unused-private-field - might require nontrivial amount of #ifdefs being added to the class declarations
-Wno-incompatible-ms-struct - we use this attribute for structs / classes that are to be marshalled between different platforms, maybe there would be a way to refactor the problematic classes to fix that
-Wno-invalid-offsetof - similar to the previous one, we might be able to refactor the problematic classes
-Wno-tautological-compare - it needs to be figured out if we can get rid of comparing this to NULL somehow.
-Wno-dangling-else - The return macro would need to be redesigned to not to use if / else to fix this problem

[janvorli]

I've created issue https://github.com/dotnet/coreclr/issues/720 for that.

[joeduffy]

Thanks everyone.

[pgavlin]

#449 and dotnet/coreclr#721 addressed some of these warnings. The out-of-tree build of LLILC using Clang on Linux is now warning free.