Skip to content

Commit

Permalink
Merge pull request #45400 from dotnet/merges/master-to-master-vs-deps
Browse files Browse the repository at this point in the history
Merge master to master-vs-deps
  • Loading branch information
msftbot[bot] authored Jun 24, 2020
2 parents 11e94bd + 9a658fa commit 1462b1e
Show file tree
Hide file tree
Showing 152 changed files with 2,507 additions and 485 deletions.
61 changes: 61 additions & 0 deletions docs/compilers/API Notes/6-11-20.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
### `BaseTypeSyntax` Design

In master, we currently have added an optional argument list to `SimpleBaseTypeSyntax` for record primary constructors. However, there is an argument to be made that we should add a new subtype of `BaseTypeSyntax`, something like `PrimaryConstructorTypeSyntax`, to represent these new base type clauses. This was an explicit extension point added in the original design for this very purpose. For either of these designs, GetSymbolInfo will behave consistently: if the `BaseTypeSyntax` has an argument list (either because it's a separate type or because it's a `PrimaryConstructorTypeSyntax`), it will return the constructor symbol or candidates as appropriate. If there are no arguments, then it will return nothing.

Pros for keeping the optional argument list on `SimpleBaseTypeSyntax`:
* The primary constructor arguments feel like an extension to existing nodes, not a totally new node type
* Existing analyzers/fixers might pick things up right without much modification

Pros for extending `BaseTypeSyntax`:
* This extension point was put in explicitly for this
* Existing code might be less likely to erroneously use it
* There was some feeling that it would serve as a more clear delineation for future additions to this syntax (if such C# features ever come to pass)

#### Conclusion

We lean towards extending `BaseTypeSyntax` in the next preview.

### `CompilationOutputFilePaths`

The plural when this struct only has one path in it currently felt wrong. Some options for renaming:

* `CompilationOutputFileInfo`
* `CompilationOutputInfo`
* `CompilationOutputFiles`

We leave it up to whoever does the rename to choose among these.

### `SolutionInfo.Create`

Add `EditorBrowsable(Never)` to the backcompat overload.

### `SymbolKind.FunctionPointer`

Suffix with `Type` for consistency with the other kinds that are also `TypeKind`s.

### `SyntaxFactory.ParseType`

The new overload should use `ParseOptions` instead of the specific C#/VB type in both factories.
Most of the Parse overloads that take a `ParseOptions` here use the non-specific one, and it can be very convenient for IDE APIs here.
While it is marginally less type-safe, the ergonomic and consistency benefits are a deciding factor.

### `WithExpressionSyntax.Receiver`

This should be renamed to `Expression` for consistency with other `SyntaxNode`s.

### `SymbolFinder.Find(Derived(Classes|Interfaces)|Interfaces)Async`

We should reconsider adding multiple overloads, and instead remove the default parameters from the existing overload and introduce a new overload with `transitive` set to `true` to match current behavior.
This is our standard modus operandi for these scenarios.

### `INegatedPatternOperation.NegatedPattern`

Rename to just `Pattern`.

### `IWithOperation.Value`

Rename to `Operand`.

### `Renamer.RenameDocumentAsync`

Make this and the related structs internal unless we have a partner team ready to consume the API in 16.7. We can make the API public again when someone is ready to consume and validate the API shape.
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ private ImmutableArray<TypedConstant> GetRewrittenAttributeConstructorArguments(
}
else if (reorderedArgument.Kind == TypedConstantKind.Array &&
parameter.Type.TypeKind == TypeKind.Array &&
!((TypeSymbol)reorderedArgument.TypeInternal).Equals(parameter.Type, TypeCompareKind.AllIgnoreOptions))
!((TypeSymbol)reorderedArgument.TypeInternal!).Equals(parameter.Type, TypeCompareKind.AllIgnoreOptions))
{
// NOTE: As in dev11, we don't allow array covariance conversions (presumably, we don't have a way to
// represent the conversion in metadata).
Expand Down Expand Up @@ -922,6 +922,7 @@ private static bool TryGetNormalParamValue(ParameterSymbol parameter, ImmutableA
}

HashSet<DiagnosticInfo>? useSiteDiagnostics = null; // ignoring, since already bound argument and parameter
Debug.Assert(argument.TypeInternal is object);
Conversion conversion = conversions.ClassifyBuiltInConversion((TypeSymbol)argument.TypeInternal, parameter.Type, ref useSiteDiagnostics);

// NOTE: Won't always succeed, even though we've performed overload resolution.
Expand Down
14 changes: 6 additions & 8 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3777,9 +3777,8 @@ private static bool IsNegativeConstantForArraySize(BoundExpression expression)
/// </summary>
/// <param name="initializerArgumentListOpt">
/// Null for implicit,
/// BaseConstructorInitializerSyntax.ArgumentList, or
/// ThisConstructorInitializerSyntax.ArgumentList, or
/// SimpleBaseTypeSyntax.ArgumentList for explicit.</param>
/// <see cref="ConstructorInitializerSyntax.ArgumentList"/>, or
/// <see cref="PrimaryConstructorBaseTypeSyntax.ArgumentList"/> for explicit.</param>
/// <param name="constructor">Constructor containing the initializer.</param>
/// <param name="diagnostics">Accumulates errors (e.g. unable to find constructor to invoke).</param>
/// <returns>A bound expression for the constructor initializer call.</returns>
Expand Down Expand Up @@ -3921,9 +3920,8 @@ private BoundExpression BindConstructorInitializerCore(
errorLocation = initializerSyntax.ThisOrBaseKeyword.GetLocation();
break;

case SimpleBaseTypeSyntax baseWithArguments:
Debug.Assert(baseWithArguments.Parent?.Parent is RecordDeclarationSyntax recordDecl && recordDecl.BaseList.Types.FirstOrDefault() == baseWithArguments);
nonNullSyntax = initializerArgumentListOpt;
case PrimaryConstructorBaseTypeSyntax baseWithArguments:
nonNullSyntax = baseWithArguments;
errorLocation = initializerArgumentListOpt.GetLocation();
break;

Expand Down Expand Up @@ -6241,9 +6239,9 @@ private BoundExpression BindInstanceMemberAccess(
lookupResult,
flags);

if (!boundMethodGroup.HasErrors && boundMethodGroup.ResultKind == LookupResultKind.Empty && typeArgumentsSyntax.Any(SyntaxKind.OmittedTypeArgument))
if (!boundMethodGroup.HasErrors && typeArgumentsSyntax.Any(SyntaxKind.OmittedTypeArgument))
{
Error(diagnostics, ErrorCode.ERR_BadArity, node, rightName, MessageID.IDS_MethodGroup.Localize(), typeArgumentsSyntax.Count);
Error(diagnostics, ErrorCode.ERR_OmittedTypeArgument, node);
}

return boundMethodGroup;
Expand Down
26 changes: 25 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ internal BoundPattern BindConstantPatternWithFallbackToTypePattern(
bool hasErrors,
DiagnosticBag diagnostics)
{
ExpressionSyntax innerExpression = expression.SkipParens();
ExpressionSyntax innerExpression = SkipParensAndNullSuppressions(expression);
if (innerExpression.Kind() == SyntaxKind.DefaultLiteralExpression)
{
diagnostics.Add(ErrorCode.ERR_DefaultPattern, innerExpression.Location);
Expand All @@ -204,6 +204,24 @@ internal BoundPattern BindConstantPatternWithFallbackToTypePattern(
}
}

private ExpressionSyntax SkipParensAndNullSuppressions(ExpressionSyntax e)
{
while (true)
{
switch (e)
{
case ParenthesizedExpressionSyntax p:
e = p.Expression;
break;
case PostfixUnaryExpressionSyntax { RawKind: (int)SyntaxKind.SuppressNullableWarningExpression } p:
e = p.Operand;
break;
default:
return e;
}
}
}

/// <summary>
/// Binds the expression for a pattern. Sets <paramref name="wasExpression"/> if it was a type rather than an expression,
/// and in that case it returns a <see cref="BoundTypeExpression"/>.
Expand Down Expand Up @@ -1239,6 +1257,12 @@ private BoundPattern BindRelationalPattern(
DiagnosticBag diagnostics)
{
BoundExpression value = BindExpressionForPattern(inputType, node.Expression, ref hasErrors, diagnostics, out var constantValueOpt, out _);
ExpressionSyntax innerExpression = SkipParensAndNullSuppressions(node.Expression);
if (innerExpression.Kind() == SyntaxKind.DefaultLiteralExpression)
{
diagnostics.Add(ErrorCode.ERR_DefaultPattern, innerExpression.Location);
hasErrors = true;
}
RoslynDebug.Assert(value.Type is { });
BinaryOperatorKind operation = tokenKindToBinaryOperatorKind(node.OperatorToken.Kind());
if (operation == BinaryOperatorKind.Equal)
Expand Down
6 changes: 2 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3322,7 +3322,7 @@ private BoundNode BindRecordConstructorBody(RecordDeclarationSyntax recordDecl,
Debug.Assert(bodyBinder != null);

BoundExpressionStatement initializer = null;
if (recordDecl.BaseWithArguments is SimpleBaseTypeSyntax baseWithArguments)
if (recordDecl.PrimaryConstructorBaseType is PrimaryConstructorBaseTypeSyntax baseWithArguments)
{
initializer = bodyBinder.BindConstructorInitializer(baseWithArguments, diagnostics);
}
Expand All @@ -3334,10 +3334,8 @@ private BoundNode BindRecordConstructorBody(RecordDeclarationSyntax recordDecl,
expressionBody: null);
}

internal BoundExpressionStatement BindConstructorInitializer(SimpleBaseTypeSyntax initializer, DiagnosticBag diagnostics)
internal virtual BoundExpressionStatement BindConstructorInitializer(PrimaryConstructorBaseTypeSyntax initializer, DiagnosticBag diagnostics)
{
Debug.Assert(initializer.Parent?.Parent is RecordDeclarationSyntax recordDecl && recordDecl.ParameterList is object && recordDecl.BaseWithArguments == initializer);

BoundExpression initializerInvocation = GetBinder(initializer).BindConstructorInitializer(initializer.ArgumentList, (MethodSymbol)this.ContainingMember(), diagnostics);
var constructorInitializer = new BoundExpressionStatement(initializer, initializerInvocation);
Debug.Assert(initializerInvocation.HasAnyErrors || constructorInitializer.IsConstructorInitializer(), "Please keep this bound node in sync with BoundNodeExtensions.IsConstructorInitializer.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ protected void FindExpressionVariables(
case SyntaxKind.GotoCaseStatement:
break;
case SyntaxKind.ArgumentList:
Debug.Assert(node.Parent is ConstructorInitializerSyntax);
Debug.Assert(node.Parent is ConstructorInitializerSyntax || node.Parent is PrimaryConstructorBaseTypeSyntax);
break;
case SyntaxKind.RecordDeclaration:
Debug.Assert(((RecordDeclarationSyntax)node).ParameterList is object);
Expand Down Expand Up @@ -397,7 +397,7 @@ public override void VisitRecordDeclaration(RecordDeclarationSyntax node)
{
Debug.Assert(node.ParameterList is object);

if (node.BaseWithArguments is SimpleBaseTypeSyntax baseWithArguments)
if (node.PrimaryConstructorBaseType is PrimaryConstructorBaseTypeSyntax baseWithArguments)
{
VisitNodeToBind(baseWithArguments);
}
Expand Down
25 changes: 15 additions & 10 deletions src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,14 @@ public override void VisitRecordDeclaration(RecordDeclarationSyntax node)

Binder enclosing = new ExpressionVariableBinder(node, _enclosing);
AddToMap(node, enclosing);
Visit(node.PrimaryConstructorBaseType, enclosing);
}

if (node.BaseWithArguments is SimpleBaseTypeSyntax baseWithArguments)
{
enclosing = enclosing.WithAdditionalFlags(BinderFlags.ConstructorInitializer);
AddToMap(baseWithArguments, enclosing);
Visit(baseWithArguments.ArgumentList, enclosing);
}
public override void VisitPrimaryConstructorBaseType(PrimaryConstructorBaseTypeSyntax node)
{
Binder enclosing = _enclosing.WithAdditionalFlags(BinderFlags.ConstructorInitializer);
AddToMap(node, enclosing);
VisitConstructorInitializerArgumentList(node, node.ArgumentList, enclosing);
}

public override void VisitDestructorDeclaration(DestructorDeclarationSyntax node)
Expand Down Expand Up @@ -317,16 +318,20 @@ public override void VisitConstructorInitializer(ConstructorInitializerSyntax no
{
var binder = _enclosing.WithAdditionalFlags(BinderFlags.ConstructorInitializer);
AddToMap(node, binder);
VisitConstructorInitializerArgumentList(node, node.ArgumentList, binder);
}

if (node.ArgumentList != null)
private void VisitConstructorInitializerArgumentList(CSharpSyntaxNode node, ArgumentListSyntax argumentList, Binder binder)
{
if (argumentList != null)
{
if (_root == node)
{
binder = new ExpressionVariableBinder(node.ArgumentList, binder);
AddToMap(node.ArgumentList, binder);
binder = new ExpressionVariableBinder(argumentList, binder);
AddToMap(argumentList, binder);
}

Visit(node.ArgumentList, binder);
Visit(argumentList, binder);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ private BoundSwitchLabel BindSwitchSectionLabel(
case SyntaxKind.CaseSwitchLabel:
{
var caseLabelSyntax = (CaseSwitchLabelSyntax)node;
SyntaxNode innerExpression = caseLabelSyntax.Value.SkipParens();
bool hasErrors = node.HasErrors;
BoundPattern pattern = sectionBinder.BindConstantPatternWithFallbackToTypePattern(
caseLabelSyntax.Value, caseLabelSyntax.Value, SwitchGoverningType, hasErrors, diagnostics);
Expand Down
58 changes: 58 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,22 @@ public static Conversion ClassifyConversion(this Compilation? compilation, IType
}
}

/// <summary>
/// Returns what symbol(s), if any, the given constructor initializer syntax bound to in the program.
/// </summary>
public static SymbolInfo GetSymbolInfo(this SemanticModel? semanticModel, PrimaryConstructorBaseTypeSyntax constructorInitializer, CancellationToken cancellationToken = default(CancellationToken))
{
var csmodel = semanticModel as CSharpSemanticModel;
if (csmodel != null)
{
return csmodel.GetSymbolInfo(constructorInitializer, cancellationToken);
}
else
{
return SymbolInfo.None;
}
}

/// <summary>
/// Returns what symbol(s), if any, the given attribute syntax bound to in the program.
/// </summary>
Expand Down Expand Up @@ -643,6 +659,27 @@ public static SymbolInfo GetSpeculativeSymbolInfo(this SemanticModel? semanticMo
}
}

/// <summary>
/// Bind the constructor initializer in the context of the specified location and get semantic information
/// about symbols. This method is used to get semantic information about a constructor
/// initializer that did not actually appear in the source code.
///
/// NOTE: This will only work in locations where there is already a constructor initializer.
/// <see cref="PrimaryConstructorBaseTypeSyntax"/>.
/// </summary>
public static SymbolInfo GetSpeculativeSymbolInfo(this SemanticModel? semanticModel, int position, PrimaryConstructorBaseTypeSyntax constructorInitializer)
{
var csmodel = semanticModel as CSharpSemanticModel;
if (csmodel != null)
{
return csmodel.GetSpeculativeSymbolInfo(position, constructorInitializer);
}
else
{
return SymbolInfo.None;
}
}

/// <summary>
/// Gets type information about a constructor initializer.
/// </summary>
Expand Down Expand Up @@ -1178,6 +1215,27 @@ public static bool TryGetSpeculativeSemanticModel([NotNullWhen(true)] this Seman
}
}

/// <summary>
/// Get a SemanticModel object that is associated with a constructor initializer that did not appear in
/// this source code. This can be used to get detailed semantic information about sub-parts
/// of a constructor initializer that did not appear in source code.
///
/// NOTE: This will only work in locations where there is already a constructor initializer.
/// </summary>
public static bool TryGetSpeculativeSemanticModel([NotNullWhen(true)] this SemanticModel? semanticModel, int position, PrimaryConstructorBaseTypeSyntax constructorInitializer, [NotNullWhen(true)] out SemanticModel? speculativeModel)
{
var csmodel = semanticModel as CSharpSemanticModel;
if (csmodel != null)
{
return csmodel.TryGetSpeculativeSemanticModel(position, constructorInitializer, out speculativeModel);
}
else
{
speculativeModel = null;
return false;
}
}

/// <summary>
/// Get a SemanticModel object that is associated with an attribute that did not appear in
/// this source code. This can be used to get detailed semantic information about sub-parts
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,9 @@
<data name="ERR_ThisInBadContext" xml:space="preserve">
<value>Keyword 'this' is not available in the current context</value>
</data>
<data name="ERR_OmittedTypeArgument" xml:space="preserve">
<value>Omitting the type argument is not allowed in the current context</value>
</data>
<data name="WRN_InvalidMainSig" xml:space="preserve">
<value>'{0}' has the wrong signature to be an entry point</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ internal override bool TryGetSpeculativeSemanticModelCore(SyntaxTreeSemanticMode
return false;
}

internal override bool TryGetSpeculativeSemanticModelCore(SyntaxTreeSemanticModel parentModel, int position, PrimaryConstructorBaseTypeSyntax constructorInitializer, out SemanticModel speculativeModel)
{
speculativeModel = null;
return false;
}

internal override bool TryGetSpeculativeSemanticModelCore(SyntaxTreeSemanticModel parentModel, int position, EqualsValueClauseSyntax initializer, out SemanticModel speculativeModel)
{
speculativeModel = null;
Expand Down
Loading

0 comments on commit 1462b1e

Please sign in to comment.