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

C# 8 nullable reference types - Preprocessor symbols for nullable reference types #1108

Closed
wants to merge 2 commits into from

Conversation

BillWagner
Copy link
Member

@BillWagner BillWagner commented May 10, 2024

Split from #1105

Fixes #1088

Separate the portion of 1105 that defines the nullable context. This includes only the preprocessor symbols, update to one introductory sample, and a related fix that updates one sample test.

@BillWagner BillWagner force-pushed the nullable-contexts branch 3 times, most recently from 71a2f3f to bdfd6f7 Compare May 10, 2024 20:26
BillWagner added a commit to BillWagner/csharpstandard that referenced this pull request May 10, 2024
Those changes have been moved to dotnet#1108
@KalleOlaviNiemitalo
Copy link
Contributor

I saw "preprocessor symbols" and thought it meant "conditional compilation symbols".

@BillWagner BillWagner marked this pull request as ready for review May 10, 2024 20:56
@BillWagner BillWagner added the meeting: discuss This issue should be discussed at the next TC49-TG2 meeting label May 10, 2024
@BillWagner
Copy link
Member Author

This should be discussed before #1105. It's shorter and simpler.

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 aspect to consider (nice and early) in terms of whether we want "one nullable context with two aspects" or "two nullable contexts".

@@ -1025,7 +1025,7 @@ right_shift_assignment

### 6.5.1 General

The pre-processing directives provide the ability to conditionally skip sections of compilation units, to report error and warning conditions, and to delineate distinct regions of source code.
The pre-processing directives provide the ability to conditionally skip sections of compilation units, to report error and warning conditions, to delineate distinct regions of source code, and to set the nullable context.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The pre-processing directives provide the ability to conditionally skip sections of compilation units, to report error and warning conditions, to delineate distinct regions of source code, and to set the nullable context.
The pre-processing directives provide the ability to conditionally skip sections of compilation units, to report error and warning conditions, to delineate distinct regions of source code, and to set the nullable contexts.

(Given that we're using the plural form later.)

Alternatively, we could decide we have a single nullable context, which has properties for both warnings and annotations.

@jskeet
Copy link
Contributor

jskeet commented May 15, 2024

Meeting notes:

  • We slightly prefer "one context, two flags" over "two contexts", but want to check with @MadsTorgersen before deciding for sure

The first commit ports in the text from dotnet#700 substantially without change. The only editorial change is to move the clause on "Nullable directives" in lexical-structure before the clause on Pragma directives.

In addition, it fixes build warnings
Remove an Xref to a section that's not in this PR.
BillWagner added a commit to BillWagner/csharpstandard that referenced this pull request May 29, 2024
Alternative PR to dotnet#1108.

This branch builds on the work in dotnet#1108. This is an alternative approach that defines a single `nullable` context. The nullable context has two properties: `annotations` and `warnings` that can be enabled or disabled separately.
@BillWagner
Copy link
Member Author

Alternative PR that defines a single nullable context is #1123.

Either this PR, or #1123 should be merged. The other should be closed.

@BillWagner
Copy link
Member Author

closing this PR in favor of #1123

@BillWagner BillWagner closed this Jul 9, 2024
@BillWagner BillWagner deleted the nullable-contexts branch July 9, 2024 21:39
BillWagner added a commit that referenced this pull request Jul 10, 2024
…llable context. (#1123)

* Port text from #700

The first commit ports in the text from #700 substantially without change. The only editorial change is to move the clause on "Nullable directives" in lexical-structure before the clause on Pragma directives.

In addition, it fixes build warnings

* Remove link (which will need to get added)

Remove an Xref to a section that's not in this PR.

* Define a single nullable context

Alternative PR to #1108.

This branch builds on the work in #1108. This is an alternative approach that defines a single `nullable` context. The nullable context has two properties: `annotations` and `warnings` that can be enabled or disabled separately.

* Update per meeting comments in review

This commit addresses all online feedback.

* Respond to feedback from June meeting

I listened to the June meeting. I updated a bit of text, and added Jon's example  from the conversation.

* Update lexical-structure.md

* Apply suggestions from code review

* Update standard/lexical-structure.md
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] Compiler directives for nullable reference types
3 participants