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

Warn/Error when a Custom Attribute does not derive from Attribute #5610

Merged
merged 2 commits into from
Sep 10, 2018

Conversation

KevinRansom
Copy link
Member

@vasily-kirichenko provide this PR: #5192

Which makes CustomAttributes which do not derive from System.Attribute an error.

Our requirements are:

  1. It must be be a warning for existing projects, which developers will see and choose to fix.
  2. An error in all new projects .NET FRamework and .NET Sdk.
  3. An Error for existing .NET Sdk projects.

Justification,
It's a breaking change and has been usable for ever on .NET Framework projects, it is however, bad practice and can cause common .Net attribute reflection idioms to not work correctly.

So on .NET framework, we want to issue a warning because the existing behavior is wrong, and developers have the opportunity to fix it.

On .NET Sdk projects, we feel that the projects were recent and still highly likely to be actively developed. This will require developers to correct the attribute definition and move on …

Kevin

@@ -1435,8 +1435,9 @@ notAFunctionButMaybeDeclaration,"This value is not a function and cannot be appl
3230,chkNoWriteToLimitedSpan,"This value can't be assigned because the target '%s' may refer to non-stack-local memory, while the expression being assigned is assessed to potentially refer to stack-local memory. This is to help prevent pointers to stack-bound memory escaping their scope."
3231,tastValueMustBeLocal,"A value defined in a module must be mutable in order to take its address, e.g. 'let mutable x = ...'"
3232,tcIsReadOnlyNotStruct,"A type annotated with IsReadOnly must also be a struct. Consider adding the [<Struct>] attribute to the type."
3233,chkStructsMayNotReturnAddressesOfContents,"Struct members cannot return the address of fields of the struct by reference"
Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I think these numbers should be returned to their original value. I have no idea how this happened I guess it was some merge nonsense.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I should, otherwise this basically undoes this change: d97c797

@KevinRansom KevinRansom force-pushed the warnoncustomattribute branch 4 times, most recently from c469583 to 71dbbc0 Compare September 9, 2018 06:41
@KevinRansom KevinRansom force-pushed the warnoncustomattribute branch 3 times, most recently from d158f07 to ac2f972 Compare September 10, 2018 20:56
@KevinRansom KevinRansom reopened this Sep 10, 2018
@KevinRansom KevinRansom reopened this Sep 10, 2018
@KevinRansom KevinRansom merged commit 8189443 into dotnet:master Sep 10, 2018
@KevinRansom KevinRansom deleted the warnoncustomattribute branch September 11, 2018 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants