Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge master to master-vs-deps #45400

Merged
8 commits merged into from
Jun 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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