Wire GetSpeculativeTypeInfo with nullability information #36833
Conversation
333fred
added
Area-Compilers
Feature - Nullable Semantic Model
There was a problem hiding this comment.
Do we actually use this property in a lot of places? Properties with big side effects make me nervous. If its only used in one or two places, consider not calling EnsureRootBoundForNullabilityIfNecessary leaving it to the caller instead, and just assert that _lazySnapshotmanager is populated. #Resolved
There was a problem hiding this comment.
It's used in derived classes, so I'd prefer to keep the field private. I will change this to a method though, so the side effects aren't unexpected.
In reply to: 298375705 [](ancestors = 298375705)
…s GetSpeculativeTypeInfo also works in a speculative semantic model.
333fred
force-pushed
the
getspeculativetypeinfo
branch
from
fd0b9f6 to
dc39452
Compare
|
@dotnet/roslyn-compiler for a second review. #Closed |
| /// <summary> | ||
| /// Only used when this is a speculative semantic model. | ||
| /// </summary> | ||
| private NullableWalker.SnapshotManager _parentSnapshotManagerOpt; |
There was a problem hiding this comment.
private [](start = 8, length = 7)
readonly #Resolved
| } | ||
|
|
||
| protected NullableWalker.SnapshotManager SnapshotManager | ||
| // We hide this property, as EnsureRootBoundForNullabilityIfNecessary can cause |
There was a problem hiding this comment.
We hide this property, as [](start = 11, length = 25)
This is a little confusing since this member is not a property and not hidden. Perhaps "Not a property because ...". #Resolved
| } | ||
|
|
||
| private static BoundExpression GetSpeculativelyBoundExpressionHelper(Binder binder, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, DiagnosticBag diagnostics) | ||
| protected static BoundExpression GetSpeculativelyBoundExpressionHelper(Binder binder, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, DiagnosticBag diagnostics) |
There was a problem hiding this comment.
protected [](start = 8, length = 9)
Is this change needed? #Resolved
4 participants
@dotnet/roslyn-compiler @chsienki for review. /cc @jasonmalinowski @ryzngard
Fixes #35037