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.

Sign up for GitHub

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

Jump to bottom

Support unmanaged constraint #24817

Merged

OmarTawfik merged 7 commits into dotnet:features/constraints from OmarTawfik:unmanaged-constraint

Feb 24, 2018

Contributor

OmarTawfik commented Feb 13, 2018 •

edited

Loading

Adds support for the unmanaged constraint in C#.

generating modreqs on the constraint, and an attribute on the type parameter.
disabled EnC for constraints.
VB treats them as bad symbols.
a following PR will handle IDE support.

OmarTawfik added 4 commits

February 12, 2018 15:39


          Source changes for unmanaged (PROTOTYPE for metadata)

0e6d4ce


          Modreqs for unmanaged as well

fb7e482


          Finished roundtripping metadata

45bd5af


          Added more tests

7bb24dc

OmarTawfik added the PR For Personal Review Only label

OmarTawfik requested review from a team as code owners

February 13, 2018 21:55

OmarTawfik removed request for a team

February 13, 2018 21:55

OmarTawfik added 2 commits

February 13, 2018 18:29


          Fix CI

53f3977


          More tests

3c47bf4

OmarTawfik changed the title ~~[WIP] Unmanaged constraint~~ Support unmanaged constraint

OmarTawfik requested review from VSadov, cston and a team

February 17, 2018 07:05

OmarTawfik added Area-Compilers Feature - Constraints and removed PR For Personal Review Only labels

OmarTawfik added this to the 15.7 milestone

Contributor Author

OmarTawfik commented Feb 17, 2018 •

edited

Loading

@dotnet/roslyn-compiler @VSadov for review #Closed

OmarTawfik removed the request for review from cston

February 17, 2018 07:09

Contributor Author

OmarTawfik commented Feb 19, 2018 •

edited

Loading

@dotnet/compiler-api for API change. #Closed


          Clean up

97d0909

Contributor Author

OmarTawfik commented Feb 21, 2018 •

edited

Loading

@dotnet/roslyn-compiler @dotnet/compiler-api @VSadov #Closed

VSadov reviewed

View reviewed changes

src/Compilers/CSharp/Test/Emit/Emit/UnmanagedTypeModifierTests.cs

+                     extends [mscorlib]System.Object
+              {
+                .method public hidebysig instance void
+                        M1<valuetype .ctor (class [mscorlib]System.ValueType modreq([mscorlib]System.Runtime.InteropServices.UnmanagedType)) T>() cil managed

Member

VSadov Feb 22, 2018 •

edited by OmarTawfik

Loading

so it looks like we honor "unmanaged" modreq even if not paired with an attribute. That seems ok to me, just wanted to be sure that this is intentional. #Closed

Contributor Author

OmarTawfik Feb 23, 2018 •

edited

Loading

@VSadov no we don't. I'll make this test more explicit (it should have the attribute).
Please check other tests that have only the modreq or the attribute and fail at both. #Closed

VSadov approved these changes

View reviewed changes

Member

VSadov left a comment

VSadov approved these changes

View reviewed changes

Member

VSadov left a comment

LGTM

Contributor Author

OmarTawfik commented Feb 23, 2018 •

edited

Loading

@dotnet/roslyn-compiler @dotnet/compiler-api can I get another sign off? #Closed

jaredpar approved these changes

View reviewed changes

jcouv assigned OmarTawfik

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PETypeParameterSymbol.cs

+                                      if (hasUnmanagedModreq != this._lazyHasIsUnmanagedAttribute.Value())
+                                      {
+                                          // The presence of UnmanagedType modreq has to match the presence of the IsUnmanagedAttribute
+                                          Interlocked.CompareExchange(ref _lazyConstraintsUseSiteErrorInfo, new CSDiagnosticInfo(ErrorCode.ERR_BindToBogus, this), CSDiagnosticInfo.EmptyErrorInfo);

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

Interlocked.CompareExchange(ref _lazyConstraintsUseSiteErrorInfo, new CSDiagnosticInfo(ErrorCode.ERR_BindToBogus, this), CSDiagnosticInfo.EmptyErrorInfo); [](start = 28, length = 154)

Why is it the right thing to report an error here? I can imagine an interface constraint combined with managed constraint, it won't be combined with the modifier. #Closed

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PETypeParameterSymbol.cs

@@ @@ -234,31 +254,44 @@ public override bool HasConstructorConstraint @@
                       {
                           get
                           {
-                              return (_flags & GenericParameterAttributes.DefaultConstructorConstraint) != 0;
+                              GetDeclaredConstraintTypes();

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

GetDeclaredConstraintTypes(); [](start = 16, length = 29)

Given my comments for GetDeclaredConstraintTypes, I don't think this change is necessary #Closed

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PETypeParameterSymbol.cs

                           }
                       }
                       public override bool HasReferenceTypeConstraint
                       {
                           get
                           {
-                              return (_flags & GenericParameterAttributes.ReferenceTypeConstraint) != 0;
+                              GetDeclaredConstraintTypes();

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

GetDeclaredConstraintTypes(); [](start = 16, length = 29)

Given my comments for GetDeclaredConstraintTypes, I don't think this change is necessary #Closed

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PETypeParameterSymbol.cs

                           }
                       }
                       public override bool HasValueTypeConstraint
                       {
                           get
                           {
-                              return (_flags & GenericParameterAttributes.NotNullableValueTypeConstraint) != 0;
+                              GetDeclaredConstraintTypes();

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

GetDeclaredConstraintTypes(); [](start = 16, length = 29)

I don't think this change is necessary #Closed

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PETypeParameterSymbol.cs

                           }
                       }
                       public override bool HasValueTypeConstraint
                       {
                           get
                           {
-                              return (_flags & GenericParameterAttributes.NotNullableValueTypeConstraint) != 0;
+                              GetDeclaredConstraintTypes();
+                              return (_lazyFlags & GenericParameterAttributes.NotNullableValueTypeConstraint) != 0 || HasUnmanagedTypeConstraint;

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

|| HasUnmanagedTypeConstraint [](start = 101, length = 29)

Presence of the attribute should not imply any constraints, whatever is specified in metadata flags should define constraints. #Closed

Member

VSadov Mar 7, 2018

Agree on this. Valid unmanaged pattern should always have valuetype constraint.
The "|| HasUnmanagedTypeConstraint" should not be needed - either there is valuetype constraint or the type is broken.

In reply to: 172413076 [](ancestors = 172413076)

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PETypeParameterSymbol.cs

+                      {
+                          get
+                          {
+                              GetDeclaredConstraintTypes();

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

GetDeclaredConstraintTypes(); [](start = 16, length = 29)

I don't think this change is necessary , presence of an attribute can be checked independently as we do everywhere else in PE symbols. #Closed

Contributor

AlekseyTs Mar 6, 2018

On the other hand, driving result of off an attribute is probably not the right thing, the custom modifier on ValueType is probably a better indicator.

In reply to: 172413190 [](ancestors = 172413190)

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PETypeParameterSymbol.cs

                           }
                       }
                       public override VarianceKind Variance
                       {
                           get
                           {
-                              return (VarianceKind)(_flags & GenericParameterAttributes.VarianceMask);
+                              GetDeclaredConstraintTypes();

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

GetDeclaredConstraintTypes(); [](start = 16, length = 29)

I don't think this change is necessary #Closed

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PETypeParameterSymbol.cs

+                                  out _,
+                                  default);
+                              this._lazyHasIsUnmanagedAttribute = (!isUnmanagedAttribute.IsNil).ToThreeState();

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

this._lazyHasIsUnmanagedAttribute = (!isUnmanagedAttribute.IsNil).ToThreeState(); [](start = 16, length = 81)

We should not rely on GetAttributes to populate this value. #Closed

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs

                               return false;
                           }
+                          if (typeParameter.HasUnmanagedTypeConstraint && (typeArgument.IsManagedType || !typeArgument.IsNonNullableValueType()))

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

typeArgument.IsManagedType || !typeArgument.IsNonNullableValueType() [](start = 61, length = 68)

It might be better to check these two requirements independently and to report separate errors for them. For example for the IsNonNullableValueType we would report the same error as in the ifbelow and for the typeArgument.IsManagedType we would say about members. Alternatively we, I think it would be better to say "must be non-nullable value type" instead of saying "cannot be a reference type", unless that is inaccurate. #Closed

Contributor Author

OmarTawfik Mar 21, 2018

I prefer the second option. I think it is great to have a separate full error message for unmanaged constraint for developers to mentally differentiate.

In reply to: 172414637 [](ancestors = 172414637)

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Emitter/Model/TypeParameterSymbolAdapter.cs

-                                                                       syntaxNodeOpt: (CSharpSyntaxNode)context.SyntaxNodeOpt,
-                                                                       diagnostics: context.Diagnostics);
+                              var typeRef = moduleBeingBuilt.Translate(
+                                  typeSymbol: moduleBeingBuilt.Compilation.GetSpecialType(SpecialType.System_ValueType),

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

moduleBeingBuilt.Compilation.GetSpecialType(SpecialType.System_ValueType) [](start = 32, length = 73)

Where do we check goodness of this type? #Closed

Contributor

AlekseyTs Mar 6, 2018

Also, it looks like below we are getting this type differently, I think we should use the same approach.

In reply to: 172414868 [](ancestors = 172414868)

Contributor Author

OmarTawfik Mar 21, 2018

Vlad changed that bit. Also we check this during binding: BindTypeParameterConstraints. We also have tests in case this type is missing.

In reply to: 172415526 [](ancestors = 172415526,172414868)

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Emitter/Model/TypeParameterSymbolAdapter.cs

-                              yield return type.GetTypeRefWithAttributes(this.DeclaringCompilation,
-                                                                         typeRef);
+                              var modifier = CSharpCustomModifier.CreateRequired(
+                                  moduleBeingBuilt.Compilation.GetWellKnownType(WellKnownType.System_Runtime_InteropServices_UnmanagedType));

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

moduleBeingBuilt.Compilation.GetWellKnownType(WellKnownType.System_Runtime_InteropServices_UnmanagedType) [](start = 19, length = 106)

Where do we check goodness of this type? #Closed

Contributor Author

OmarTawfik Mar 21, 2018

We check this during binding: BindTypeParameterConstraints. We also have tests in case this type is missing.

In reply to: 172415132 [](ancestors = 172415132)

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Emitter/Model/TypeParameterSymbolAdapter.cs

+                              var modifier = CSharpCustomModifier.CreateRequired(
+                                  moduleBeingBuilt.Compilation.GetWellKnownType(WellKnownType.System_Runtime_InteropServices_UnmanagedType));
+                              yield return new Cci.TypeReferenceWithAttributes(new Cci.ModifiedTypeReference(typeRef, ImmutableArray.Create<Cci.ICustomModifier>(modifier)));

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

yield return new Cci.TypeReferenceWithAttributes(new Cci.ModifiedTypeReference(typeRef, ImmutableArray.Create<Cci.ICustomModifier>(modifier))); [](start = 16, length = 143)

Why are we not emitting other constraint types? For example, interfaces? #Closed

Contributor Author

OmarTawfik Mar 21, 2018

Vlad changed that in is PR.

In reply to: 172415271 [](ancestors = 172415271)

Contributor Author

OmarTawfik Mar 21, 2018

Will add a couple of tests as well.

In reply to: 175987954 [](ancestors = 175987954,172415271)

Contributor

AlekseyTs commented Mar 6, 2018 •

edited

Loading

            return this.HasConstructorConstraint || this.HasValueTypeConstraint;

Why are we not implying MustHaveDefaultConstructor for HasUnmanagedTypeConstraint? We are implying MustBeValueType. #Closed

Refers to: src/Compilers/CSharp/Portable/Emitter/Model/TypeParameterSymbolAdapter.cs:295 in 97d0909. [](commit_id = 97d0909, deletion_comment = False)

AlekseyTs reviewed

View reviewed changes

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

@@ @@ -133,55 +135,80 @@ internal partial class Binder @@
                                       {
                                           var typeConstraintSyntax = (TypeConstraintSyntax)syntax;
                                           var typeSyntax = typeConstraintSyntax.Type;
-                                          if (typeSyntax.Kind() != SyntaxKind.PredefinedType && !SyntaxFacts.IsName(typeSyntax.Kind()))
+                                          var typeSyntaxKind = typeSyntax.Kind();
+                                          if (typeSyntaxKind != SyntaxKind.PredefinedType && typeSyntaxKind != SyntaxKind.PointerType && !SyntaxFacts.IsName(typeSyntax.Kind()))

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

typeSyntaxKind != SyntaxKind.PointerType [](start = 79, length = 40)

It is not obvious why we are no longer reporting this error for pointers. Could you elaborate? #Closed

Contributor Author

OmarTawfik Mar 21, 2018

Added a comment. This error is reported for pointers during binding typeSyntax (to prevent cycles). So we don't need a duplicate error here.

In reply to: 172416549 [](ancestors = 172416549)

AlekseyTs reviewed

View reviewed changes

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

-                                          }
+                                              if (constraints != 0 || constraintTypes.Any())
+                                              {
+                                                  diagnostics.Add(ErrorCode.ERR_UnmanagedConstraintMustBeAlone, typeSyntax.GetLocation());

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

diagnostics.Add(ErrorCode.ERR_UnmanagedConstraintMustBeAlone, typeSyntax.GetLocation()); [](start = 36, length = 88)

Why can't it be combined with an interface? #Closed

Member

VSadov Mar 7, 2018

It should be able to combine with interfaces. I think we discussed this on the tesplan review as a potential hole/followup issue.

In reply to: 172416658 [](ancestors = 172416658)

Contributor Author

OmarTawfik Mar 21, 2018

Vlad took care of that.

In reply to: 173018531 [](ancestors = 173018531,172416658)

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs

                                       foreach (var typeParameter in this.TypeParameters)
                                       {
                                           typeParameter.ForceComplete(locationOpt, cancellationToken);
+                                          var diagnostics = DiagnosticBag.GetInstance();

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

var diagnostics = DiagnosticBag.GetInstance(); [](start = 28, length = 46)

It feels like all this added code belongs in Force complete for the type parameter. Looks like CompletionPart.TypeParameterConstraints is the relevant part. #Closed

Contributor Author

OmarTawfik Mar 21, 2018

Since we decided to enable local functions, the last parameter would be false. Will add that now.

In reply to: 172417776 [](ancestors = 172417776)

Contributor

AlekseyTs Mar 26, 2018

All added code belongs in typeParameter.ForceComplete?

In reply to: 175998462 [](ancestors = 175998462,172417776)

Contributor Author

OmarTawfik Mar 26, 2018

Consolidated into the other comment.

In reply to: 177238909 [](ancestors = 177238909,175998462,172417776)

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs

+                          foreach (var typeParameter in _typeParameters)
+                          {
+                              if (typeParameter.HasUnmanagedTypeConstraint)

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

if (typeParameter.HasUnmanagedTypeConstraint) [](start = 16, length = 45)

This check belong in completion for the type parameter, this feels like a wrong place for this code. #Closed

Contributor Author

OmarTawfik Mar 21, 2018

Since we decided to enable local functions, the last parameter would be false. Will add that now.

In reply to: 172418067 [](ancestors = 172418067)

Contributor

AlekseyTs Mar 26, 2018

I thin the added code belongs in completion for the type parameter

In reply to: 175998583 [](ancestors = 175998583,172418067)

Contributor Author

OmarTawfik Mar 26, 2018

Consolidated into the other comment.

In reply to: 177244069 [](ancestors = 177244069,175998583,172418067)

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs

+                              if (typeParam.HasUnmanagedTypeConstraint)
+                              {
+                                  addTo.Add(ErrorCode.ERR_UnmanagedConstraintWithLocalFunctions, typeParam.GetNonNullSyntaxNode().Location);

Contributor

AlekseyTs Mar 6, 2018 •

edited

Loading

addTo.Add(ErrorCode.ERR_UnmanagedConstraintWithLocalFunctions, typeParam.GetNonNullSyntaxNode().Location); [](start = 20, length = 106)

It feels like typeParam.ForceComplete is the right place to report this error. #Closed

Contributor Author

OmarTawfik Mar 21, 2018

Will remove since we decided to enable local functions.

In reply to: 172418267 [](ancestors = 172418267)

Contributor

AlekseyTs Mar 26, 2018

Belongs in typeParam.ForceComplete?

In reply to: 175998657 [](ancestors = 175998657,172418267)

Contributor Author

OmarTawfik Mar 26, 2018

Consolidated into the other comment.

In reply to: 177237879 [](ancestors = 177237879,175998657,172418267)

AlekseyTs reviewed

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Wrapped/WrappedTypeParameterSymbol.cs

@@ @@ -73,6 +73,14 @@ public override bool HasReferenceTypeConstraint @@
                           }
                       }
+                      public override bool HasUnmanagedTypeConstraint

Contributor

AlekseyTs Mar 6, 2018 •

edited by OmarTawfik

Loading

public override bool HasUnmanagedTypeConstraint [](start = 8, length = 47)

Do we have tests covering all subclasses of the enclosing class? #Resolved

Contributor Author

OmarTawfik Mar 21, 2018

can you please clarify? which scenario do you see uncovered by this property?

In reply to: 172419029 [](ancestors = 172419029)

Contributor

AlekseyTs Mar 21, 2018

can you please clarify? which scenario do you see uncovered by this property?

I asked the question whether you covered all leaf classes derived from this one in tests. Did you?

In reply to: 175999068 [](ancestors = 175999068,172419029)

Contributor Author

OmarTawfik Mar 28, 2018

Added tests for retargeting and non-synthesized type parameters. We already have tests for synthesized ones.

In reply to: 176269706 [](ancestors = 176269706,175999068,172419029)

Contributor

AlekseyTs Mar 28, 2018

Added tests for retargeting and non-synthesized type parameters. We already have tests for synthesized ones.

What about SubstitutedTypeParameterSymbol?

In reply to: 177881475 [](ancestors = 177881475,176269706,175999068,172419029)

Contributor

AlekseyTs commented Mar 6, 2018 •

edited by OmarTawfik

Loading

@OmarTawfik I believe there is an actionable feedback for this PR, even though it has been already merged. #Resolved

OmarTawfik mentioned this pull request

Follow up on feeback on #24817 #25261

Closed

Contributor Author

OmarTawfik commented Mar 6, 2018 •

edited

Loading

@AlekseyTs I'm currently OOF. Created #25261 so that I don't forget to follow up when I get back :) #Closed

Member

VSadov commented Mar 7, 2018

            return this.HasConstructorConstraint || this.HasValueTypeConstraint;

This is arguable. Emitting .ctor for struct constraint is a dubious practice that we carry oved from old compiler.
.ctor is redundant in the presence of valuetype since all structs satisfy .ctor constraint.

In reply to: 370671892 [](ancestors = 370671892)

Refers to: src/Compilers/CSharp/Portable/Emitter/Model/TypeParameterSymbolAdapter.cs:295 in 97d0909. [](commit_id = 97d0909, deletion_comment = False)

Member

VSadov commented Mar 7, 2018

            return this.HasConstructorConstraint || this.HasValueTypeConstraint;

I guess, we might want to extend the practice to unmanaged for "consistency". If we mean unmanaged as enhanced struct, then extra constraints should be purely additive.

In reply to: 371325898 [](ancestors = 371325898,370671892)

Refers to: src/Compilers/CSharp/Portable/Emitter/Model/TypeParameterSymbolAdapter.cs:295 in 97d0909. [](commit_id = 97d0909, deletion_comment = False)

Member

VSadov commented Mar 7, 2018

            return this.HasConstructorConstraint || this.HasValueTypeConstraint;

I wonder if we want to require .ctor when parsing unmanaged. It kind of does not make sense to enforce. I guess we should treat it the same as for general struct.

In reply to: 371326199 [](ancestors = 371326199,371325898,370671892)

Refers to: src/Compilers/CSharp/Portable/Emitter/Model/TypeParameterSymbolAdapter.cs:295 in 97d0909. [](commit_id = 97d0909, deletion_comment = False)

Member

VSadov commented Mar 7, 2018

            return this.HasConstructorConstraint || this.HasValueTypeConstraint;

If we require .ctor, the change must be made before merging or we may have metadata compat breaks later.

In reply to: 371326436 [](ancestors = 371326436,371326199,371325898,370671892)

Refers to: src/Compilers/CSharp/Portable/Emitter/Model/TypeParameterSymbolAdapter.cs:295 in 97d0909. [](commit_id = 97d0909, deletion_comment = False)

Member

VSadov commented Mar 8, 2018

            return this.HasConstructorConstraint || this.HasValueTypeConstraint;

We accept valuetype without either .ctor or ValueType. We have tests for that.

In reply to: 371327262 [](ancestors = 371327262,371326436,371326199,371325898,370671892)

Refers to: src/Compilers/CSharp/Portable/Emitter/Model/TypeParameterSymbolAdapter.cs:295 in 97d0909. [](commit_id = 97d0909, deletion_comment = False)

AlekseyTs reviewed

View reviewed changes

src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs

+                                          }
+                                          else if (!modifiers.IsDefaultOrEmpty)
+                                          {
+                                              // Optional modifiers (also, other required parameters) not allowed on generic parameters

Contributor

AlekseyTs Mar 19, 2018 •

edited

Loading

parameters [](start = 76, length = 10)

What parameters does this refer to? #Closed

Contributor Author

OmarTawfik Mar 20, 2018

Bad phrase. Will fix.

In reply to: 175516830 [](ancestors = 175516830)

AlekseyTs reviewed

View reviewed changes

src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs

+                                          }
+                                          else if (!modifiers.IsDefaultOrEmpty)
+                                          {
+                                              // Optional modifiers (also, other required parameters) not allowed on generic parameters

Contributor

AlekseyTs Mar 19, 2018 •

edited

Loading

not allowed on generic parameters [](start = 88, length = 33)

This function as about constraints rather than about generic type parameters #Closed

OmarTawfik commented

View reviewed changes

src/Compilers/CSharp/Test/Emit/Emit/UnmanagedTypeModifierTests.cs

+              namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Emit
+              {
+                  public class UnmanagedTypeModifierTests : CSharpTestBase

Contributor Author

OmarTawfik Mar 28, 2018 •

edited

Loading

UnmanagedTypeModifierTests [](start = 17, length = 26)

test with best betternes when features are combined. - Since we can resolve overloading on constraints now. #Resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers Feature - Constraints