Skip to content

Add error for multiple null suppressions#37139

Merged
agocke merged 1 commit intodotnet:masterfrom
agocke:error-on-multiple-suppress
Jul 11, 2019
Merged

Add error for multiple null suppressions#37139
agocke merged 1 commit intodotnet:masterfrom
agocke:error-on-multiple-suppress

Conversation

@agocke
Copy link
Copy Markdown
Member

@agocke agocke commented Jul 11, 2019

Fixes #37111

@agocke agocke requested a review from jcouv July 11, 2019 02:56
@agocke agocke marked this pull request as ready for review July 11, 2019 06:22
@agocke agocke requested a review from a team as a code owner July 11, 2019 06:22
// G(s!!);
Diagnostic(ErrorCode.ERR_DuplicateNullSuppression, "s").WithLocation(6, 11),
// (7,12): error CS8715: Duplicate null suppression operator ('!')
// G((s!)!);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(s!)! [](start = 29, length = 5)

Does this need to be an error?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is pretty much just reserving the right to consider !! a distinct operator in the future, right? So probably no need to complain in this scenario.

Copy link
Copy Markdown
Member

@jcouv jcouv Jul 11, 2019

Choose a reason for hiding this comment

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

I expected we would implement this as a parsing error: expr!! would be an error, but (expr!)! and expr! ! would not.
I wouldn't push back very hard on this given the timing though, so up to you.


In reply to: 302638675 [](ancestors = 302638675)

Copy link
Copy Markdown
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM but I would notify Mads/LDM of the implemented behavior, which differs from my expectation.
Thanks (iteration 1)

@jcouv jcouv added the Feature - Nullable Reference Types Nullable Reference Types label Jul 11, 2019
@jcouv jcouv self-assigned this Jul 11, 2019
@jcouv jcouv added this to the 16.3.P2 milestone Jul 11, 2019
@agocke
Copy link
Copy Markdown
Member Author

agocke commented Jul 11, 2019

I think this should be a semantic error for now. We can always relax it later.

@agocke agocke merged commit 9ee1532 into dotnet:master Jul 11, 2019
@agocke agocke deleted the error-on-multiple-suppress branch July 11, 2019 20:20
333fred added a commit to 333fred/roslyn that referenced this pull request Jul 11, 2019
* dotnet/master:
  Fix master build (dotnet#37163)
  Add error for multiple null suppressions (dotnet#37139)
  Check for uninitialized non-nullable field-like events (dotnet#37073)
  EnC emit tests for C# 8.0 patterns (dotnet#37025)
  Update dependencies from https://github.com/dotnet/arcade build 20190710.8
  Update dependencies from https://github.com/dotnet/arcade build 20190709.6
  [master] Update dependencies from dotnet/arcade (dotnet#37001)
  Add annotations to HostWorkspaceServices.cs and HostLanguageServices.cs
  Add nullable annotations to members in Solution.cs, SolutionInfo.cs, and SolutionState.cs
  Annotate Contract.cs
  Add nullable annotations and fixes to Workspace.cs
  Fix typo in documentation comment for Workspace.Dispose
  Check in copy of NullableAttributes.cs copied from coreclr
  Use switch expressions to simplify GetAccessibilityAndModifiers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Block expr!! (double suppression)

4 participants