Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

build-test - fix ilasm warnings caused by missing/incorrect extern assembly declarations #19188

Merged
merged 2 commits into from
Aug 1, 2018

Conversation

4creators
Copy link

@4creators 4creators commented Jul 28, 2018

Fixes #14689

While working on #19109 fixed all ilasm warnings for priority 0 and 1 builds.

Since my editor is automatically enforcing .editorconfig coding style white space was fixed in all files which were edited (tabs -> spaces, no white space at the end of line)

Will create separate issue to track not fixed #14689 warnings - MSB3268

@4creators 4creators changed the title build-test - fix ildasm warnings due to missing extern assembly declarations build-test - fix ilasm warnings caused by missing/incorrect extern assembly declarations Jul 28, 2018
@4creators
Copy link
Author

Related issue #19174

@4creators
Copy link
Author

4creators commented Jul 29, 2018

Failures of OSX10.12 x64 Checked Build and Test (Jit - TieredCompilation=0 JitStress=2 (JitStressRegs=3) ) are unrelated to this PR and tracked by #19190

@4creators
Copy link
Author

@jkotas @CarolEidt @AndyAyersMS PTAL

@CarolEidt
Copy link

I don't believe that we should need to put explicit publickeytoken specifications in the IL tests (or at least not for most of them). I wonder if these are here simply because the original IL was created from ildasm, and in many cases it should probably be using .assembly extern BLAH { auto }.

@4creators
Copy link
Author

Will fix

@4creators
Copy link
Author

4creators commented Aug 1, 2018

@CarolEidt Addressed review feedback PTAL

Assembly declarations have been corrected with script, if you think we can do it to all il source files I could run script for all of them. Please let me now.

…n assembly declarations

white space was fixed in all files which were edited (tabs -> spaces, no white space at the end of line)
@CarolEidt
Copy link

@dotnet-bot test OSX10.12 x64 Checked CoreFX Tests
@dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test

@CarolEidt
Copy link

if you think we can do it to all il source files I could run script for all of them. Please let me now.

I don't think it's necessary. I don't know if we have any tests that explicitly test the use of non-auto assembly declarations.

@4creators
Copy link
Author

@CarolEidt I will replace my "Remove unnecessry assembly declaration metadata" with new version with some improvements to white space and mscorlib handling.

Copy link

@CarolEidt CarolEidt 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!

@CarolEidt CarolEidt merged commit 2293351 into dotnet:master Aug 1, 2018
@4creators 4creators deleted the test-warnings-il branch August 4, 2018 10:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Many CI Perf Tests Correctness Warnings: about targeting .NETFramework,Version=v4.0
2 participants