Skip to content

Not poisoning restricted types known to be stack only.#22275

Merged
VSadov merged 2 commits into
dotnet:masterfrom
VSadov:refFixes001
Sep 21, 2017
Merged

Not poisoning restricted types known to be stack only.#22275
VSadov merged 2 commits into
dotnet:masterfrom
VSadov:refFixes001

Conversation

@VSadov
Copy link
Copy Markdown
Member

@VSadov VSadov commented Sep 21, 2017

Not poisoning restricted types known to be stack only. (as per ECMA 335)

  • TypedReference
  • ArgIterator
  • RuntimeArgumentHandle

Fixes:#22198

===
There are technical reasons for which making RestrictedTypes ref structs is advantageous. In fact it is already done in some cases - see example: https://github.com/dotnet/corert/blob/f23fb52ce14fa680bcebc07eab9a20fa8780e47f/src/ILCompiler.TypeSystem/tests/CoreTestAssembly/Platform.cs#L71

We do not need to poison these types with Obsolete attribute when ref struct syntax is used. The stack-only semantics of these types is widely known and poisoning can only lead to compat troubles.

TypedReference
ArgIterator
RuntimeArgumentHandle

Fixes:#dotnet#22198
@VSadov
Copy link
Copy Markdown
Member Author

VSadov commented Sep 21, 2017

@dotnet/roslyn-compiler - please take a look

@VSadov
Copy link
Copy Markdown
Member Author

VSadov commented Sep 21, 2017

CC @jaredpar - for ask mode approval.

}

[Fact()]
public void NoTypedRefBox2()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How does this differ from NoTypedRefBox1? Consider adding a comment.

@VSadov
Copy link
Copy Markdown
Member Author

VSadov commented Sep 21, 2017

Microsoft.CodeAnalysis.Editor.UnitTests.Structure.StructureTaggerTests.CSharpOutliningTagger_RegionIsNotDefinition failed windows_debug_unit32_prtest.

  • could not be related to this change.

@VSadov
Copy link
Copy Markdown
Member Author

VSadov commented Sep 21, 2017

test windows_debug_unit32_prtest please

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants