Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reenable some disabled warnings in CoreCLR #34659

Merged
merged 15 commits into from
Apr 9, 2020
Merged

Reenable some disabled warnings in CoreCLR #34659

merged 15 commits into from
Apr 9, 2020

Conversation

ivdiazsa
Copy link
Member

@ivdiazsa ivdiazsa commented Apr 7, 2020

This is a followup to PR #33902, which has been reviewed and approved. The reason for this new PR, is that Azure Pipelines tests got stuck and the solution was to get the latest changes. When rebasing the branch with master, all those commits and changes were added to the PR, thus rendering it entirely uninformative, and therefore useless for historic study and statistics.

Fixes #33743.

Added some necessary casts and fixed string formats to comply with warnings 4302, 4311, 4312, and 4477 in CoreCLR. Having these warnings back on will help improve code quality and reduce potential scenarios where unpredicted behavior might cause further issues.

@ivdiazsa ivdiazsa requested a review from trylek April 7, 2020 22:01
@ivdiazsa ivdiazsa changed the title Reenable some disbaled warnings in CoreCLR Reenable some disabled warnings in CoreCLR Apr 7, 2020
Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks Ivan for your tenacity in fighting GitHub quirks! Please just make sure to use a squash merge to spare us the details of your fight ;-).

@janvorli
Copy link
Member

janvorli commented Apr 8, 2020

There are still some warnings that cause build break in the GCC build:

2020-04-07T23:09:31.6969516Z In file included from /__w/1/s/src/coreclr/src/jit/codegeninterface.h:27:0,
2020-04-07T23:09:31.6971238Z                  from /__w/1/s/src/coreclr/src/jit/compiler.h:52,
2020-04-07T23:09:31.6972337Z                  from /__w/1/s/src/coreclr/src/jit/jit.h:800,
2020-04-07T23:09:31.6977378Z                  from /__w/1/s/src/coreclr/src/jit/jitpch.h:19,
2020-04-07T23:09:31.6978600Z                  from /__w/1/s/src/coreclr/src/jit/alloc.cpp:5:
2020-04-07T23:09:31.7071934Z /__w/1/s/src/coreclr/src/jit/emit.h:522:51: warning: 'emitter::emitAddrMode::amBaseReg' is too small to hold all values of 'regNumber {aka enum _regNumber_enum}'
2020-04-07T23:09:31.7073513Z          regNumber       amBaseReg : REGNUM_BITS + 1;
2020-04-07T23:09:31.7074589Z                                                    ^

@ivdiazsa
Copy link
Member Author

ivdiazsa commented Apr 8, 2020

There are still some warnings that cause build break in the GCC build:
2020-04-07T23:09:31.6969516Z In file included from /__w/1/s/src/coreclr/src/jit/codegeninterface.h:27:0,
2020-04-07T23:09:31.6971238Z from /__w/1/s/src/coreclr/src/jit/compiler.h:52,
2020-04-07T23:09:31.6972337Z from /__w/1/s/src/coreclr/src/jit/jit.h:800,
2020-04-07T23:09:31.6977378Z from /__w/1/s/src/coreclr/src/jit/jitpch.h:19,
2020-04-07T23:09:31.6978600Z from /__w/1/s/src/coreclr/src/jit/alloc.cpp:5:
2020-04-07T23:09:31.7071934Z /__w/1/s/src/coreclr/src/jit/emit.h:522:51: warning: 'emitter::emitAddrMode::amBaseReg' is too small to hold all values of 'regNumber {aka enum _regNumber_enum}'
2020-04-07T23:09:31.7073513Z regNumber amBaseReg : REGNUM_BITS + 1;
2020-04-07T23:09:31.7074589Z ^

Those are new! Let me take a look and fix them. Thanks for pointing them out Jan.

@ivdiazsa ivdiazsa closed this Apr 8, 2020
@ivdiazsa ivdiazsa deleted the warning_compliance_code branch April 8, 2020 18:57
@ivdiazsa ivdiazsa restored the warning_compliance_code branch April 8, 2020 18:58
@ivdiazsa ivdiazsa reopened this Apr 8, 2020
@ivdiazsa ivdiazsa merged commit 62d8baf into dotnet:master Apr 9, 2020
@ivdiazsa ivdiazsa deleted the warning_compliance_code branch April 9, 2020 20:58
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reenable Warnings 4302, 4311, 4312, and 4377 in CoreCLR
3 participants