delete enum support for bool as an underlying type by pedrobsaila · Pull Request #79962 · dotnet/runtime

pedrobsaila · 2022-12-25T16:48:05Z

ghost · 2022-12-25T16:48:19Z

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

Author:	pedrobsaila
Assignees:	-
Labels:	`area-System.Runtime`
Milestone:	-

vargaz · 2022-12-28T03:45:29Z

The mono changes look ok to me.

stephentoub

Changes LGTM. @jkotas, your suggestion was about considering removing bool; we later got data confirming it's very rarely (but not 100% never) used. Still want to nuke it?

jkotas · 2023-01-06T01:39:50Z


+### II.14.3 Enums (page 168)
+
+Enum support for bool as underlying type was deleted (see [issue 79224](https://github.com/dotnet/runtime/issues/79224)).


This change is deleting the bool backed enum support from the reflection stack.
The bool backed enums are still going to work loaded fine by the type loader. I think that it is the right tradeoff between compat and simplicity. The type loader support for bool backed enums is very cheap and simple.
ECMA-335 and this doc is concerned about the behavior of the core runtime (that is not changing), not about behavior of BCL APIs. I think this note should be deleted.

What is the 'type loader' ?

Type loader is a component responsible for loading types.
For example, Console.WriteLine(typeof(BoolEnum)); is still going to work fine even with these changes because the type loader still works fine for bool enums with these changes.

jkotas · 2023-01-06T01:42:51Z

Still want to nuke it?

Yes, I think it is fine to nuke it from parsing, formatting and conversions. I would keep it in the core type loader - it does not cost anything there.

jkotas

LGTM. Thank you!

akoeplinger · 2023-01-13T17:12:50Z

Can we update https://github.com/dotnet/runtime/blob/main/docs/design/specs/Ecma-335-Augments.md with this?

stephentoub · 2023-01-13T17:14:47Z

Can we update https://github.com/dotnet/runtime/blob/main/docs/design/specs/Ecma-335-Augments.md with this?

https://github.com/dotnet/runtime/pull/79962/files/a8bd8383cb3a54a4864de06d077ac967fa7a6aab#diff-30929ad6de186623e8903e6516a190745da62d62df407351f1efee396cfafd7b

akoeplinger · 2023-01-13T18:08:55Z

ah ok, thanks

ghost · 2023-01-20T02:21:18Z

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

Partial revert of dotnet#79962Fixesdotnet#80890

jkotas · 2023-01-20T04:46:56Z

Breaking change doc issue dotnet/docs#33647

Partial revert of #79962Fixes#80890

Partial revert of dotnet#79962Fixesdotnet#80890

jkotas · 2023-10-16T17:15:49Z

Breaking change doc created by dotnet/docs#36505

delete enum support for bool as an underlying type
ad91e56

pedrobsaila requested a review from MichalStrehovsky as a code owner December 25, 2022 16:48

ghost added area-System.Runtime community-contribution Indicates that the PR has been added by a community member labels Dec 25, 2022

fix failing unit tests 1
f01ed0a

dakersnar requested a review from stephentoub December 27, 2022 16:24

forgotten boolean enum code
3bcdf7a

pedrobsaila requested review from lambdageek, thaystg and vargaz as code owners December 27, 2022 22:33

build-analysisBot mentioned this pull request Dec 28, 2022
Tracking issue for CI build timeouts #76454
Closed

pedrobsaila added 2 commits December 29, 2022 22:14

fix failing tests
a07cb54

fix failing tests in system.runtime
9a6bf2a

build-analysisBot mentioned this pull request Dec 30, 2022
emcc received SIGKILL #79874
Closed

stephentoub reviewed Jan 3, 2023
View reviewed changes

Comment threadsrc/libraries/System.Runtime/tests/System/EnumTests.cs

stephentoub approved these changes Jan 3, 2023
View reviewed changes

pedrobsaila added 3 commits January 5, 2023 22:13

add test for boolean enum building
2503ca5

delete useless new line
72486cc

fix unit tests
a8bd838

jkotas reviewed Jan 6, 2023
View reviewed changes

Comment threadsrc/libraries/Microsoft.VisualBasic.Core/tests/ConversionsTests.cs

pedrobsaila added 2 commits January 7, 2023 22:58

keep loading bool backed enum tests
4f56b30

forgotten code
c2809e3

jkotas reviewed Jan 9, 2023
View reviewed changes

Comment threadsrc/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs Outdated

pedrobsaila added 2 commits January 9, 2023 22:09

rollback changes in the type loader
5c597b1

rollback additionaml test
61aa895

jkotas reviewed Jan 10, 2023
View reviewed changes

Comment threadsrc/libraries/System.Private.CoreLib/src/System/Enum.cs

jkotas reviewed Jan 10, 2023
View reviewed changes

Comment threadsrc/libraries/System.Private.CoreLib/src/System/Enum.cs

rollback code optimized by jit
965f278

jkotas approved these changes Jan 10, 2023
View reviewed changes

Merge branch 'main' into 79224
ce14b49

jkotas merged commit f32ba52 into dotnet:mainJan 13, 2023

pedrobsaila deleted the 79224 branch January 13, 2023 08:30

stephentoub added breaking-change Issue or PR that represents a breaking API or functional change over a previous release. needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet labels Jan 20, 2023

jkotas added a commit to jkotas/runtime that referenced this pull request Jan 20, 2023
Fix Enum.ToObject for bool to Enum
0ebea2e
Partial revert of dotnet#79962Fixesdotnet#80890

jkotas mentioned this pull request Jan 20, 2023
Fix Enum.ToObject for bool to Enum #80893
Merged

stephentoub pushed a commit that referenced this pull request Jan 20, 2023
Fix Enum.ToObject for bool to Enum (#80893)
ce934fd
Partial revert of #79962Fixes#80890

mdh1418 pushed a commit to mdh1418/runtime that referenced this pull request Jan 24, 2023
Fix Enum.ToObject for bool to Enum (dotnet#80893)
7012da2
Partial revert of dotnet#79962Fixesdotnet#80890

jaredpar mentioned this pull request Jan 30, 2023
Consider allowing native integer types as enum underlying types dotnet/roslyn#44110
Open

ghost locked as resolved and limited conversation to collaborators Feb 19, 2023

ericstj added this to the 8.0.0 milestone Jun 28, 2023

ericstj assigned jkotas Oct 16, 2023

jkotas removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 16, 2023


		### II.14.3 Enums (page 168)

		Enum support for bool as underlying type was deleted (see [issue 79224](https://github.com/dotnet/runtime/issues/79224)).

Uh oh!

Conversation

pedrobsaila commented Dec 25, 2022

Uh oh!

ghost commented Dec 25, 2022

Uh oh!

vargaz commented Dec 28, 2022

Uh oh!

Uh oh!

stephentoub left a comment

Choose a reason for hiding this comment

Uh oh!

jkotasJan 6, 2023

Choose a reason for hiding this comment

Uh oh!

pedrobsailaJan 7, 2023

Choose a reason for hiding this comment

Uh oh!

jkotasJan 7, 2023

Choose a reason for hiding this comment

Uh oh!

jkotas commented Jan 6, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkotas left a comment

Choose a reason for hiding this comment

Uh oh!

akoeplinger commented Jan 13, 2023

Uh oh!

stephentoub commented Jan 13, 2023

Uh oh!

akoeplinger commented Jan 13, 2023

Uh oh!

ghost commented Jan 20, 2023• edited by jkotas Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkotas commented Jan 20, 2023

Uh oh!

jkotas commented Oct 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jkotas commented Jan 6, 2023•
edited
Loading

ghost commented Jan 20, 2023•
edited by jkotas
Loading