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

Include AttributeUsageAttribute with synthesized nullable attributes #36958

Merged
merged 2 commits into from Jul 10, 2019

Conversation

Projects
None yet
4 participants
@cston
Copy link
Member

commented Jul 3, 2019

Include [AttributeUsage(…, Inherited=false)] for synthesized nullable attributes so NullableAttribute and NullableContextAttribute are not interpreted as inherited.

For instance, if a base class has [NullableContext(2)], that attribute should apply to the base type only, and not to derived types that have no explicit NullableContextAttribute.

Fixes #36934

@cston cston requested a review from dotnet/roslyn-compiler as a code owner Jul 3, 2019

@cston

This comment has been minimized.

Copy link
Member Author

commented Jul 3, 2019

@dotnet/roslyn-compiler please review.

@333fred

333fred approved these changes Jul 3, 2019

Copy link
Member

left a comment

LGTM (commit 2)

@RikkiGibson RikkiGibson requested a review from dotnet/roslyn-compiler Jul 5, 2019

@cston cston force-pushed the cston:36934 branch from 1c6ba60 to 96ea527 Jul 9, 2019

@cston

This comment has been minimized.

Copy link
Member Author

commented Jul 9, 2019

@dotnet/roslyn-compiler, please review.

@jcouv

jcouv approved these changes Jul 9, 2019

Copy link
Member

left a comment

LGTM Thanks (iteration 2)

@jcouv

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

It would be good to add some notes in this PR documenting the sort of impact the bug had, in case people run into it and we need to point them to a summary.

Tagging @stephentoub @safern as FYI (the attributes emitted by the compiler didn't have the proper Inherited flag). I don't remember whether corefx relies on attributes embedded by the compiler or on its own definition of the attributes...

@cston cston merged commit 3272de5 into dotnet:master Jul 10, 2019

16 checks passed

WIP Ready for review
Details
license/cla All CLA requirements met.
Details
roslyn-CI Build #20190708.57 succeeded
Details
roslyn-CI (Linux_Test coreclr) Linux_Test coreclr succeeded
Details
roslyn-CI (Linux_Test mono) Linux_Test mono succeeded
Details
roslyn-CI (MacOs_Test) MacOs_Test succeeded
Details
roslyn-CI (Windows_CoreClr_Unit_Tests debug) Windows_CoreClr_Unit_Tests debug succeeded
Details
roslyn-CI (Windows_CoreClr_Unit_Tests release) Windows_CoreClr_Unit_Tests release succeeded
Details
roslyn-CI (Windows_Correctness_Test) Windows_Correctness_Test succeeded
Details
roslyn-CI (Windows_Desktop_Spanish_Unit_Tests) Windows_Desktop_Spanish_Unit_Tests succeeded
Details
roslyn-CI (Windows_Desktop_Unit_Tests debug_32) Windows_Desktop_Unit_Tests debug_32 succeeded
Details
roslyn-CI (Windows_Desktop_Unit_Tests debug_64) Windows_Desktop_Unit_Tests debug_64 succeeded
Details
roslyn-CI (Windows_Desktop_Unit_Tests release_32) Windows_Desktop_Unit_Tests release_32 succeeded
Details
roslyn-CI (Windows_Desktop_Unit_Tests release_64) Windows_Desktop_Unit_Tests release_64 succeeded
Details
roslyn-CI (Windows_Determinism_Test) Windows_Determinism_Test succeeded
Details
roslyn-integration-CI Build #20190708.57 succeeded
Details

@cston cston deleted the cston:36934 branch Jul 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.