Skip to content

Replace dynamic with object when substituting constraints.#36379

Merged
AlekseyTs merged 2 commits intodotnet:masterfrom
AlekseyTs:DynamicConstraint_01
Jun 14, 2019
Merged

Replace dynamic with object when substituting constraints.#36379
AlekseyTs merged 2 commits intodotnet:masterfrom
AlekseyTs:DynamicConstraint_01

Conversation

@AlekseyTs
Copy link
Copy Markdown
Contributor

@AlekseyTs AlekseyTs commented Jun 12, 2019 •
edited
Loading

Fixes #36276.

@AlekseyTs AlekseyTs marked this pull request as ready for review June 12, 2019 22:57
@AlekseyTs AlekseyTs requested a review from a team as a code owner June 12, 2019 22:57
@jcouv
Copy link
Copy Markdown
Member

jcouv commented Jun 12, 2019 •
edited by AlekseyTs
Loading 

            Assert.Equal("void Test1<dynamic>.M1<S>()", baseM1.ToDisplayString(SymbolDisplayFormat.TestFormatWithConstraints));

Object~ constraints are omitted, so that's why we don't see a where S : Object~ constraint here. Is that right? #Resolved

Refers to: src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs:70292 in 2a68337. [](commit_id = 2a683375dbbebf5c250f453760205e9e8804772f, deletion_comment = False)

jcouv
jcouv reviewed Jun 12, 2019
View reviewed changes
src/Compilers/CSharp/Portable/Symbols/AbstractTypeMap.cs Outdated
Copy link
Copy Markdown
Member

@jcouv jcouv Jun 12, 2019 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/// Substitute types, and return the results without duplicates, preserving the original order. [](start = 8, length = 95)

May be worth adding a comment regarding dynamic. #Resolved

jcouv
jcouv approved these changes Jun 12, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new commit (iteration 7) LGTM. Thanks

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

AlekseyTs commented Jun 12, 2019 

            Assert.Equal("void Test1<dynamic>.M1<S>()", baseM1.ToDisplayString(SymbolDisplayFormat.TestFormatWithConstraints));

Object~ constraints are omitted, so that's why we don't see a where S : Object~ constraint here. Is that right?

That is correct. Insignificant Object constraints are dropped.

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

Refers to: src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs:70292 in 2a68337. [](commit_id = 2a683375dbbebf5c250f453760205e9e8804772f, deletion_comment = False)

@jcouv jcouv added this to the 16.2.P4 milestone Jun 12, 2019
@jcouv jcouv self-assigned this Jun 12, 2019
@AlekseyTs
Copy link
Copy Markdown
Contributor Author

AlekseyTs commented Jun 13, 2019 •
edited
Loading

@cston Please review the last commit.

@AlekseyTs AlekseyTs force-pushed the DynamicConstraint_01 branch from 2a68337 to 597d042 Compare June 13, 2019 17:43
@AlekseyTs
Copy link
Copy Markdown
Contributor Author

AlekseyTs commented Jun 13, 2019

@dotnet/roslyn-compiler, @cston Please review, need a second sign-off.

@gafter
Copy link
Copy Markdown
Member

gafter commented Jun 13, 2019

Is this a change that reflects something that is or should be in the specification?

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

AlekseyTs commented Jun 13, 2019

Is this a change that reflects something that is or should be in the specification?

I do not think so.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

AlekseyTs commented Jun 14, 2019

@dotnet/roslyn-compiler, @cston Please review, need a second sign-off.

@AlekseyTs AlekseyTs merged commit 9e3c4b8 into dotnet:master Jun 14, 2019
333fred added a commit to 333fred/roslyn that referenced this pull request Jun 18, 2019
@333fred
Merge remote-tracking branch 'dotnet/master' into null-coalesce-value…
23153de 
…-types

* dotnet/master: (63 commits)
  Fix stack overflow in requesting syntax directives (dotnet#36347)
  crash on ClassifyUpdate for EventFields (dotnet#35962)
  Disable move type when the options service isn't present (dotnet#36334)
  Fix crash where type inference doing method inference needs to drop nullability
  Fix parsing bug in invalid using statements (dotnet#36428)
  Do not suggest or diagnose use compound assignment when right hand of binary operator is a throw expression
  Add option to emit nullable metadata for public members only (dotnet#36398)
  Added null checks on F# external access services (dotnet#36469)
  Deal with discovering extra .editorconfig files
  Re-enable MSBuildWorkspaceTests.TestEditorConfigDiscovery
  Add support to VisualStudioMSBuildInstalled to support minimum versions
  Fix configuration of accessibilities in editorconfig
  Shorten a resource ID
  Revert "Extract the RDT implementation for Misc files and VS open file tracker"
  Add nullability support to use local function
  Add EditorFeatures.WPF dependency to F# ExternalAccess
  Ensure NullableWalker.AsMemberOfType locates the right new container for the member. (dotnet#36406)
  Replace `dynamic` with `object` when substituting constraints. (dotnet#36379)
  Add some string descriptions
  Adjust type of out var based on parameter state (dotnet#36284)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jcouv jcouv jcouv approved these changes

+1 more reviewer

@cston cston cston approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jcouv jcouv

Labels

Area-Compilers Feature - Nullable Reference Types Nullable Reference Types

Projects

None yet

Milestone

16.2.P4

Development

Successfully merging this pull request may close these issues.

Inconsistent handling of dynamic generic type constraint when overriding is involved.

4 participants

@AlekseyTs @jcouv @gafter @cston