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

Add the list of attributes that support nullable analysis. #1191

Open
wants to merge 14 commits into
base: draft-v8
Choose a base branch
from

Conversation

BillWagner
Copy link
Member

Fixes #1092

Based on the last meeting, list the attributes so that their function is normative, however a compiler need not issues warnings based on these rules.

Include all the nullable analysis attributes in the spec. This will be edited heavily and shrunk down to a smaller set of normative text.
Copy link
Contributor

@Nigel-Ecma Nigel-Ecma left a comment

Choose a reason for hiding this comment

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

An early review, just comments. In general I’m not sure the must/may language is correct. I haven’t gone through all the examples so no comments there but a glance suggests I'd have similar queries – but examples are informative…

standard/attributes.md Outdated Show resolved Hide resolved
standard/attributes.md Outdated Show resolved Hide resolved
@BillWagner BillWagner marked this pull request as ready for review October 24, 2024 20:15
@BillWagner BillWagner added the meeting: discuss This issue should be discussed at the next TC49-TG2 meeting label Oct 24, 2024
Copy link
Contributor

@Nigel-Ecma Nigel-Ecma left a comment

Choose a reason for hiding this comment

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

Just another bunch of comments

standard/attributes.md Outdated Show resolved Hide resolved
standard/attributes.md Outdated Show resolved Hide resolved
standard/attributes.md Outdated Show resolved Hide resolved
standard/attributes.md Outdated Show resolved Hide resolved
standard/attributes.md Outdated Show resolved Hide resolved
standard/attributes.md Outdated Show resolved Hide resolved
standard/attributes.md Outdated Show resolved Hide resolved
standard/attributes.md Show resolved Hide resolved
standard/attributes.md Outdated Show resolved Hide resolved
Copy link
Contributor

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

One more suggested change, but basically fine. (Thanks!)

standard/attributes.md Show resolved Hide resolved
standard/attributes.md Outdated Show resolved Hide resolved
Copy link
Contributor

@Nigel-Ecma Nigel-Ecma left a comment

Choose a reason for hiding this comment

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

A couple more comments

standard/attributes.md Outdated Show resolved Hide resolved
standard/attributes.md Outdated Show resolved Hide resolved
> }
> ```
>
> The presence of the attribute helps the compiler in a number of ways. First, the compiler can issue a warning if there's a path where the method can exit without throwing an exception. Second, the compiler can consider any code after a call to that method as unreachable, until an appropriate catch clause is found. Third, the unreachable code won't affect any null states. *end example*
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be reworded. The compiler doesn't use this attribute for definite assignment nor reachability. However, it will only supress any nullability.

When the `DoesNotReturn` attribute is parsed by a compiler that provides nullable diagnostics, that attribute can't impact reachable code analysis.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meeting: discuss This issue should be discussed at the next TC49-TG2 meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Nullable Reference Types] Specify behavior for nullable analysis attributes
3 participants