Skip to content

Commit

Permalink
Implements the initial set of span-safety checks.
Browse files Browse the repository at this point in the history
These are mostly the checks dealing with stack-only nature of the Span. With minor difference they follow the same logic as the existing checks for types such as TypedReference.

==== Not in this change:
Defining and detecting span-like types is NYI. For now we just treat any type named "System.Span" and "System.ReadonlySpan" as span-like. This will change.

Some of the checks result in somewhat generaic messages and happen at emit phase. That was ok when the failures were supposed to be rare.
Error clarity is not the goal of this change, but we will examone what errors should say and whether they should be moved to an earlier phase.
  • Loading branch information
VSadov committed Apr 6, 2017
1 parent 7add7ea commit c67384d
Show file tree
Hide file tree
Showing 12 changed files with 648 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,13 @@ private static Conversion ToConversion(OverloadResolutionResult<MethodSymbol> re
return Conversion.NoConversion;
}

//PROTOTYPE(span): generalize to all restricted types or it would be a compat break?
//cannot capture span-like types.
if (!method.IsStatic && methodGroup.Receiver.Type.IsSpanLikeType())
{
return Conversion.NoConversion;
}

if (method.OriginalDefinition.ContainingType.SpecialType == SpecialType.System_Nullable_T &&
!method.IsOverride)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ private void MakeFrames(ArrayBuilder<ClosureDebugInfo> closureDebugInfo)
proxies.Add(captured, new CapturedToFrameSymbolReplacement(hoistedField, isReusable: false));
CompilationState.ModuleBuilderOpt.AddSynthesizedDefinition(frame, hoistedField);

//PROTOTYPE(span): move the check to the binding?
if (hoistedField.Type.IsRestrictedType())
{
foreach (CSharpSyntaxNode syntax in _analysis.CapturedVariables[captured])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,8 @@ private static void ReportParameterErrors(
Location loc = parameterSyntax.Identifier.GetNextToken(includeZeroWidth: true).GetLocation(); //could be missing
diagnostics.Add(ErrorCode.ERR_DefaultValueBeforeRequiredValue, loc);
}
else if (parameter.RefKind != RefKind.None && parameter.Type.IsRestrictedType())
else if (parameter.RefKind != RefKind.None &&
parameter.Type.IsRestrictedType(ignoreSpanLikeTypes: parameter.RefKind == RefKind.RefReadOnly))
{
// CS1601: Cannot make reference to variable of type 'System.TypedReference'
diagnostics.Add(ErrorCode.ERR_MethodArgCantBeRefAny, parameterSyntax.Location, parameter.Type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ internal static void AddDelegateMembers(
var objectType = binder.GetSpecialType(SpecialType.System_Object, diagnostics, syntax);
var intPtrType = binder.GetSpecialType(SpecialType.System_IntPtr, diagnostics, syntax);

if (returnType.IsRestrictedType())
if (returnType.IsRestrictedType(ignoreSpanLikeTypes: true))
{
// Method or delegate cannot return type '{0}'
diagnostics.Add(ErrorCode.ERR_MethodReturnCantBeRefAny, returnTypeSyntax.Location, returnType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ protected void TypeChecks(TypeSymbol type, DiagnosticBag diagnostics)
{
diagnostics.Add(ErrorCode.ERR_FieldCantHaveVoidType, TypeSyntax.Location);
}
//PROTOTYPE(span): span-like instance fields are allowed in span-like types, for now disallow always
else if (type.IsRestrictedType())
{
diagnostics.Add(ErrorCode.ERR_FieldCantBeRefAny, TypeSyntax.Location, type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ private void MethodChecks(MethodDeclarationSyntax syntax, Binder withTypeParamsB
var returnTypeSyntax = syntax.ReturnType.SkipRef(out refKind);
_lazyReturnType = signatureBinder.BindType(returnTypeSyntax, diagnostics);

if (_lazyReturnType.IsRestrictedType())
// span-like types are returnable in general
if (_lazyReturnType.IsRestrictedType(ignoreSpanLikeTypes: true))
{
if (_lazyReturnType.SpecialType == SpecialType.System_TypedReference &&
(this.ContainingType.SpecialType == SpecialType.System_TypedReference || this.ContainingType.SpecialType == SpecialType.System_ArgIterator))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1298,7 +1298,7 @@ internal override void ForceComplete(SourceLocation locationOpt, CancellationTok
var conversions = new TypeConversions(this.ContainingAssembly.CorLibrary);
this.Type.CheckAllConstraints(conversions, _location, diagnostics);

if (this.Type.IsRestrictedType())
if (this.Type.IsRestrictedType(ignoreSpanLikeTypes: !this.IsAutoProperty))
{
diagnostics.Add(ErrorCode.ERR_FieldCantBeRefAny, this.CSharpSyntaxNode.Type.Location, this.Type);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ protected override void MethodChecks(DiagnosticBag diagnostics)

_lazyReturnType = signatureBinder.BindType(ReturnTypeSyntax, diagnostics);

if (_lazyReturnType.IsRestrictedType())
// restricted types cannot be returned.
// NOTE: Span-like types can be returned (if expression is returnable).
if (_lazyReturnType.IsRestrictedType(ignoreSpanLikeTypes: true))
{
// Method or delegate cannot return type '{0}'
diagnostics.Add(ErrorCode.ERR_MethodReturnCantBeRefAny, ReturnTypeSyntax.Location, _lazyReturnType);
Expand Down
25 changes: 25 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,31 @@ public virtual NamedTypeSymbol TupleUnderlyingType
/// </remarks>
internal abstract bool IsManagedType { get; }

//PROTOTYPE(span): this will completely change depending on how span-like types are implemented.
// Span<T> and ReadOnlySpan<T> will be special types (currently they are not always)
// Users may be able to define their own Spanlike types, but that is completely NYI
// For now we will be simply looking at "System.Span" and "System.ReadOnlySpan" names
/// <summary>
/// Returns true if the type is a Span/ReadOnlySpan
/// </summary>
internal bool IsSpanLikeType()
{
var originalDef = this.OriginalDefinition;

if (originalDef.Name != "Span" && originalDef.Name != "ReadonlySpan")
{
return false;
}

var ns = originalDef.ContainingSymbol as NamespaceSymbol;
if (ns?.Name != "System")
{
return false;
}

return (object)ns.ContainingNamespace != null;
}

#region ITypeSymbol Members

INamedTypeSymbol ITypeSymbol.BaseType
Expand Down
9 changes: 7 additions & 2 deletions src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -917,9 +917,11 @@ internal static bool IsValidV6SwitchGoverningType(this TypeSymbol type, bool isT
/// <summary>
/// Returns true if the type is one of the restricted types, namely: <see cref="T:System.TypedReference"/>,
/// <see cref="T:System.ArgIterator"/>, or <see cref="T:System.RuntimeArgumentHandle"/>.
/// or a ref-like type.
/// </summary>
#pragma warning restore RS0010
internal static bool IsRestrictedType(this TypeSymbol type)
internal static bool IsRestrictedType(this TypeSymbol type,
bool ignoreSpanLikeTypes = false)
{
// See Dev10 C# compiler, "type.cpp", bool Type::isSpecialByRefType() const
Debug.Assert((object)type != null);
Expand All @@ -930,7 +932,10 @@ internal static bool IsRestrictedType(this TypeSymbol type)
case SpecialType.System_RuntimeArgumentHandle:
return true;
}
return false;

return ignoreSpanLikeTypes?
false:
type.IsSpanLikeType();
}

public static bool IsIntrinsicType(this TypeSymbol type)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@
<Compile Include="Semantics\InteractiveUsingTests.cs" />
<Compile Include="Semantics\ScriptSemanticsTests.cs" />
<Compile Include="Semantics\SemanticAnalyzerTests.cs" />
<Compile Include="Semantics\SpanStackSafetyTests.cs" />
<Compile Include="Semantics\SemanticErrorTests.cs" />
<Compile Include="Semantics\StructsTests.cs" />
<Compile Include="Semantics\SuppressAccessibilityChecksTests.cs" />
Expand Down
Loading

0 comments on commit c67384d

Please sign in to comment.