Address issues with nullable flow analysis slot container types and c…

…hanging types during inference (#39301)

Fixes #39220
Fixes #39297
Fixes #33428

src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs

@@ -8180,7 +8180,7 @@ private BoundConditionalAccess GenerateBadConditionalAccessNodeError(Conditional
  // conditional access is not really a binary operator.
  DiagnosticInfo diagnosticInfo = new CSDiagnosticInfo(ErrorCode.ERR_BadUnaryOp, SyntaxFacts.GetText(operatorToken.Kind()), access.Display);
  diagnostics.Add(new CSDiagnostic(diagnosticInfo, operatorToken.GetLocation()));
- access = BadExpression(access.Syntax, access);
+ receiver = BadExpression(receiver.Syntax, receiver);
 
  return new BoundConditionalAccess(node, receiver, access, CreateErrorType(), hasErrors: true);
  }

src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs

@@ -968,7 +968,8 @@ protected BoundLocalDeclaration BindVariableDeclaration(
  // Check for variable declaration errors.
  // Use the binder that owns the scope for the local because this (the current) binder
  // might own nested scope.
- bool hasErrors = localSymbol.ScopeBinder.ValidateDeclarationNameConflictsInScope(localSymbol, diagnostics);
+ bool nameConflict = localSymbol.ScopeBinder.ValidateDeclarationNameConflictsInScope(localSymbol, diagnostics);
+ bool hasErrors = false;
 
  var containingMethod = this.ContainingMemberOrLambda as MethodSymbol;
  if (containingMethod != null && containingMethod.IsAsync && localSymbol.RefKind != RefKind.None)
@@ -1145,10 +1146,10 @@ protected BoundLocalDeclaration BindVariableDeclaration(
  syntax: associatedSyntaxNode,
  localSymbol: localSymbol,
  declaredTypeOpt: boundDeclType,
- initializerOpt: hasErrors ? BindToTypeForErrorRecovery(initializerOpt) : initializerOpt,
+ initializerOpt: hasErrors ? BindToTypeForErrorRecovery(initializerOpt)?.WithHasErrors() : initializerOpt,
  argumentsOpt: arguments,
  inferredType: isVar,
- hasErrors: hasErrors);
+ hasErrors: hasErrors | nameConflict);
  }
 
  internal ImmutableArray<BoundExpression> BindDeclaratorArguments(VariableDeclaratorSyntax declarator, DiagnosticBag diagnostics)

src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs

@@ -314,20 +314,43 @@ private void MakeTestsAndBindings(
  tests.Add(valueAsITupleEvaluation);
  var valueAsITuple = new BoundDagTemp(syntax, iTupleType, valueAsITupleEvaluation);
 
- var lengthEvaluation = new BoundDagPropertyEvaluation(syntax, getLengthProperty, valueAsITuple);
+ var lengthEvaluation = new BoundDagPropertyEvaluation(syntax, getLengthProperty, OriginalInput(valueAsITuple, getLengthProperty));
  tests.Add(lengthEvaluation);
  var lengthTemp = new BoundDagTemp(syntax, this._compilation.GetSpecialType(SpecialType.System_Int32), lengthEvaluation);
  tests.Add(new BoundDagValueTest(syntax, ConstantValue.Create(patternLength), lengthTemp));
 
+ var getItemPropertyInput = OriginalInput(valueAsITuple, getItemProperty);
  for (int i = 0; i < patternLength; i++)
  {
- var indexEvaluation = new BoundDagIndexEvaluation(syntax, getItemProperty, i, valueAsITuple);
+ var indexEvaluation = new BoundDagIndexEvaluation(syntax, getItemProperty, i, getItemPropertyInput);
  tests.Add(indexEvaluation);
  var indexTemp = new BoundDagTemp(syntax, objectType, indexEvaluation);
  MakeTestsAndBindings(indexTemp, pattern.Subpatterns[i].Pattern, tests, bindings);
  }
  }
 
+ /// <summary>
+ /// Get the earliest input of which the symbol is a member.
+ /// A BoundDagTypeEvaluation doesn't change the underlying object being pointed to.
+ /// So two evaluations act on the same input so long as they have the same original input.
+ /// We use this method to compute the original input for an evaluation.
+ /// </summary>
+ private BoundDagTemp OriginalInput(BoundDagTemp input, Symbol symbol)
+ {
+ while (input.Source is BoundDagTypeEvaluation source && IsDerivedType(source.Input.Type, symbol.ContainingType))
+ {
+ input = source.Input;
+ }
+
+ return input;
+ }
+
+ bool IsDerivedType(TypeSymbol possibleDerived, TypeSymbol possibleBase)
+ {
+ HashSet<DiagnosticInfo> useSiteDiagnostics = null;
+ return this._conversions.HasIdentityOrImplicitReferenceConversion(possibleDerived, possibleBase, ref useSiteDiagnostics);
+ }
+
  private void MakeTestsAndBindings(
  BoundDagTemp input,
  BoundDeclarationPattern declaration,
@@ -451,7 +474,7 @@ private void MakeTestsAndBindings(
  if (recursive.DeconstructMethod != null)
  {
  MethodSymbol method = recursive.DeconstructMethod;
- var evaluation = new BoundDagDeconstructEvaluation(recursive.Syntax, method, input);
+ var evaluation = new BoundDagDeconstructEvaluation(recursive.Syntax, method, OriginalInput(input, method));
  tests.Add(evaluation);
  int extensionExtra = method.IsStatic ? 1 : 0;
  int count = Math.Min(method.ParameterCount - extensionExtra, recursive.Deconstruction.Length);
@@ -481,7 +504,7 @@ private void MakeTestsAndBindings(
  BoundPattern pattern = recursive.Deconstruction[i].Pattern;
  SyntaxNode syntax = pattern.Syntax;
  FieldSymbol field = elements[i];
- var evaluation = new BoundDagFieldEvaluation(syntax, field, input); // fetch the ItemN field
+ var evaluation = new BoundDagFieldEvaluation(syntax, field, OriginalInput(input, field)); // fetch the ItemN field
  tests.Add(evaluation);
  var output = new BoundDagTemp(syntax, field.Type, evaluation);
  MakeTestsAndBindings(output, pattern, tests, bindings);
@@ -508,10 +531,10 @@ private void MakeTestsAndBindings(
  switch (symbol)
  {
  case PropertySymbol property:
- evaluation = new BoundDagPropertyEvaluation(pattern.Syntax, property, input);
+ evaluation = new BoundDagPropertyEvaluation(pattern.Syntax, property, OriginalInput(input, property));
  break;
  case FieldSymbol field:
- evaluation = new BoundDagFieldEvaluation(pattern.Syntax, field, input);
+ evaluation = new BoundDagFieldEvaluation(pattern.Syntax, field, OriginalInput(input, field));
  break;
  default:
  Debug.Assert(recursive.HasAnyErrors);

src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
+using System.Threading;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.PooledObjects;
@@ -51,7 +52,7 @@ internal ConversionsBase WithNullability(bool includeNullability)
  }
  if (_lazyOtherNullability == null)
  {
- _lazyOtherNullability = WithNullabilityCore(includeNullability);
+ Interlocked.CompareExchange(ref _lazyOtherNullability, WithNullabilityCore(includeNullability), null);
  }
  Debug.Assert(_lazyOtherNullability.IncludeNullability == includeNullability);
  Debug.Assert(_lazyOtherNullability._lazyOtherNullability == this);

src/Compilers/CSharp/Portable/BoundTree/BoundDagEvaluation.cs

@@ -16,7 +16,7 @@ public virtual bool Equals([NotNullWhen(true)] BoundDagEvaluation? other)
  {
  return !(other is null) &&
  this.Kind == other.Kind &&
- this.GetOriginalInput().Equals(other.GetOriginalInput()) &&
+ this.Input.Equals(other.Input) &&
  this.Symbol.Equals(other.Symbol, TypeCompareKind.AllIgnoreOptions);
  }
  private Symbol Symbol
@@ -37,23 +37,7 @@ private Symbol Symbol
 
  public override int GetHashCode()
  {
- return Hash.Combine(GetOriginalInput().GetHashCode(), Symbol.GetHashCode());
- }
-
- /// <summary>
- /// Returns the original input for this evaluation, stripped of all Type Evaluations.
- /// 
- /// A BoundDagTypeEvaluation doesn't change the underlying object being pointed to
- /// So two evaluations act on the same input so long as they have the same original input.
- /// </summary>
- private BoundDagTemp GetOriginalInput()
- {
- var input = this.Input;
- while (input.Source is BoundDagTypeEvaluation source)
- {
- input = source.Input;
- }
- return input;
+ return Hash.Combine(Input.GetHashCode(), this.Symbol?.GetHashCode() ?? 0);
  }
 
  public static bool operator ==(BoundDagEvaluation? left, BoundDagEvaluation? right)

src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs

@@ -67,6 +67,9 @@ public sealed partial class CSharpCompilation : Compilation
  private ImmutableArray<Diagnostic> _lazyClsComplianceDiagnostics;
 
  private Conversions? _conversions;
+ /// <summary>
+ /// A conversions object that ignores nullability.
+ /// </summary>
  internal Conversions Conversions
  {
  get

src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.VariableIdentifier.cs

@@ -82,7 +82,7 @@ public bool Equals(VariableIdentifier other)
  return true;
  }
 
- return Symbol.OriginalDefinition.Equals(other.Symbol.OriginalDefinition, SymbolEqualityComparer.ConsiderEverything.CompareKind);
+ return Symbol.Equals(other.Symbol, TypeCompareKind.AllIgnoreOptions);
  }
 
  public override bool Equals(object obj)

src/Compilers/CSharp/Portable/FlowAnalysis/LocalDataFlowPass.cs

@@ -107,7 +107,7 @@ protected virtual bool IsEmptyStructType(TypeSymbol type)
  /// <summary>
  /// Force a variable to have a slot. Returns -1 if the variable has an empty struct type.
  /// </summary>
- protected int GetOrCreateSlot(Symbol symbol, int containingSlot = 0, bool forceSlotEvenIfEmpty = false)
+ protected virtual int GetOrCreateSlot(Symbol symbol, int containingSlot = 0, bool forceSlotEvenIfEmpty = false)
  {
  Debug.Assert(containingSlot >= 0);
 
@@ -228,7 +228,8 @@ protected Symbol GetNonMemberSymbol(int slot)
  VariableIdentifier variableId = variableBySlot[slot];
  while (variableId.ContainingSlot > 0)
  {
- Debug.Assert(variableId.Symbol.Kind == SymbolKind.Field || variableId.Symbol.Kind == SymbolKind.Property || variableId.Symbol.Kind == SymbolKind.Event);
+ Debug.Assert(variableId.Symbol.Kind == SymbolKind.Field || variableId.Symbol.Kind == SymbolKind.Property || variableId.Symbol.Kind == SymbolKind.Event,
+ "inconsistent property symbol owner");
  variableId = variableBySlot[variableId.ContainingSlot];
  }
  return variableId.Symbol;
@@ -286,6 +287,7 @@ protected int MakeMemberSlot(BoundExpression receiverOpt, Symbol member)
  {
  containingSlot = 0;
  }
+
  return GetOrCreateSlot(member, containingSlot);
  }