Skip to content

Adjust doc comment for NullableWalker.VisitConversion#53429

Merged
RikkiGibson merged 2 commits intomainfrom
dev/rigibson/adjust-doc
May 18, 2021
Merged

Adjust doc comment for NullableWalker.VisitConversion#53429
RikkiGibson merged 2 commits intomainfrom
dev/rigibson/adjust-doc

Conversation

@RikkiGibson
Copy link
Member

I felt like it reads better to put the description of various flags into <param> tags. Also added a description for bool useLegacyWarnings. Not interested in renaming it at this point, but I feel like it is useful to capture somewhere what exactly it does.

/// If <see langword="true"/>, the incoming conversion is assumed to be from binding
/// and will be re-calculated, this time considering nullability.
/// Note that the conversion calculation considers nested nullability only.
/// The caller is responsible for checking the top-level nullability of
Copy link
Member

Choose a reason for hiding this comment

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

may I ask what do you mean by top-level nullablity? is it the outermost/initial/current state?

Copy link
Member Author

Choose a reason for hiding this comment

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

The conversion from List<string?> to List<string> has a nested nullability violation, while the conversion from List<string>? to List<string>, has a top-level nullability violation.

@RikkiGibson
Copy link
Member Author

Please take a look when you have the chance @cston.

@jcouv jcouv self-assigned this May 18, 2021
Copy link
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 Thanks (iteration 1)

@jcouv jcouv added this to the 17.0 milestone May 18, 2021
@RikkiGibson RikkiGibson merged commit 6d8cdc1 into main May 18, 2021
@ghost ghost modified the milestones: 17.0, Next May 18, 2021
@RikkiGibson RikkiGibson deleted the dev/rigibson/adjust-doc branch May 18, 2021 01:48
333fred added a commit to 333fred/roslyn that referenced this pull request May 20, 2021
…vice-featureslayer

* upstream/main: (857 commits)
  Update contrib documentation (dotnet#53504)
  SImplify
  Fix out of bound crash in lsp navto.
  Revert changes to TypeScriptWaitContext wrappers
  Switch to ROSLYN_TEST_CI for CI detection
  SImplify
  Simplify LoggerTestChannel using BlockingCollection
  Only require telemetry validation in CI
  Fix out of bound crash in lsp navto.
  Fix locked comment
  Update Compiler Test Plan.md (dotnet#53420)
  Adjust doc comment for NullableWalker.VisitConversion (dotnet#53429)
  Revert "Infer delegate types with -langversion:preview only (dotnet#53241)" (dotnet#53466)
  Fix syntax normalizer to add space around before colon in constructor initializer (dotnet#53326)
  Remove unnecessary property (dotnet#53406)
  EnC - Tell the debugger about updated type def tokens (dotnet#53217)
  Revert an error
  Update PublishData.json
  Keep trailing trivia so single line if statements don't get badly formatted (dotnet#53414)
  Fix dead test code (dotnet#53416)
  ...
@RikkiGibson RikkiGibson modified the milestones: Next, 17.0.P2 Jun 29, 2021
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.

4 participants