From 6f41b7f5a2956a45e3a8ada62727595e6e735114 Mon Sep 17 00:00:00 2001 From: AdamSpeight2008 Date: Sun, 7 Jan 2018 12:21:00 +0000 Subject: [PATCH 1/5] Initial draft of Flags Enum Operators proposal. --- proposals/proposal-FlagsEnumOperators.md | 153 +++++++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 proposals/proposal-FlagsEnumOperators.md diff --git a/proposals/proposal-FlagsEnumOperators.md b/proposals/proposal-FlagsEnumOperators.md new file mode 100644 index 0000000..5f5be11 --- /dev/null +++ b/proposals/proposal-FlagsEnumOperators.md @@ -0,0 +1,153 @@ +# Feature: Flags Enum Operators + +* [x] Proposed +* [ ] Prototype: [Complete](https://github.com/PROTOTYPE_OWNER/roslyn/BRANCH_NAME) +* [ ] Implementation: [In Progress](https://github.com/dotnet/roslyn/BRANCH_NAME) +* [ ] Specification: [Not Started](pr/1) + +## Summary +[summary]: #summary + +Simplifyy the usage of flags enums. + +## Motivation +[motivation]: #motivation + +Imagine we are using the follow set of enum flags. +```vbnet + +Enum Flags + Node = 0 + Red = 1 + Green = 2 + Blue = 4 +End Enum +``` +* There are four basic operations we do on flags + * **IsSet** + * `IsSet = (MyFlags And Flags.Red) = Flags.Red` + * **IsClear** + * `IsClr = (MyFlags And Flags.Red) <> Flags.Red` + * **Set** + * `MyFlags = (MyFlags Or Flags.Red)` + * **Clear** + * `MyFlags = (MyFlags And Not Flags.Red)` + +It would good if we could define a set of generic extension methods that implement those functions. + +```vbnet +Imports System.Runtime.CompilerServices + +Public Module FlagsEnumExts + + + Public Function IsSet(Of Fe As Enum)( Flags As Fe, Flag As Fe) As Boolean + Return (Flags And Flag) = Flag + End Function + + + Public Function IsClr(Of Fe As Enum)( Flags As Fe, Flag As Fe) As Boolean + Return (Flags And Flag) <> Flag + End Function + + + Public Function [Set](Of Fe As Enum)( Flags As Fe, Flag As Fe) As Fe + Return (Flags Or Flag) + End Function + + Public Function Clr(Of Fe As Enum)( Flags As Fe, Flag As Fe) As Fe + Return (Flags And Not Flag) + End Function + +End Module +``` + +Unfortunatly this isn't currently possible in VB.net (as of VB15.5). +We could us a non generic implementation, but that incurs a runtime reflection cost. + + +## Detailed design +[design]: #detailed-design + +### Overview of the operators + +This is proposal introduces the concept of a **Flags Enum Operator**. +A Flags Enum operator consist of three parts + * `Flags` + * The flags enum (or variable of a flags enum) being work on. + * `!` + * The operation being performed. + * 'flag` + * The flag to work with. + * Note: `flag` is a literal referencing the member of the flags to use. + * Note: It isn't a reference to variable. + +Initially one will be supported + * `flags!flag` + * This borrows syntax from them dictionary lookup operator `!`. + * This usage is analogous to `IsSet`, those returns a `Boolean`. + * `IsClr` is supported via negation `Not flags!flag` or comparing against `flags!flag = false`. + +Two other additional operator, are to be consider also;= + * `flags!+flag` + * This is analogous to `[Set](Of Fe As Enum)(flags As Fe, Flag As Fe) As Fe` + * `flags!-flag` + * This is analogous to `Clr(Of Fe As Enum)(flags As Fe, Flag) As Fe` + * These two operators will have to special case in the compiler. + * As not to mean;- + * `!+` `(flags : Single) + DirectCast(flag : Fe.UnderlayingType)` + * `!-` `(flags : Single) - DirectCast(flag : Fe.UnderlayingType)` + respectively. + * Ie an *identifier* followed by the *single_type_character* + +All of the operators act immutablly, returning a new instance of the flags enum. + + +Example code: + +```VB.NET +' ... +Dim myFlags = Flags.None +Dim flagsA = myFlags!+Red!+Green!+Blue +Dim flagsB = flagsA!-Green +' Explict comparision used for clarity. +If flagsA!Green = True Then + ' ... +ElseIf flagsB!Green = True Then + ' ... +End If +' ... +``` + +### Parsing + +Should be relatively simple to extend existing support for dictionary lookup operator. + +Existing usage `Dim results = Flags!Flag` results in an error. +`Error BC30103 '!' requires its left operand to have a type parameter, class or interface type, but this operand has the type 'Main.Flags` + +So could extended under a feature flags, to preserve compatability. + +The operator `!+` and `!-` will require additional parsing code to support. + +## Advantages + + * Simplified usage. + * Compiler-Time verification of supported enums. *As they are essentially constants.* + +## Drawbacks +[drawbacks]: #drawbacks + +Why should we *not* do this? + +## Alternatives +[alternatives]: #alternatives + +What other designs have been considered? What is the impact of not doing this? + +## Unresolved questions +[unresolved]: #unresolved-questions + +What parts of the design are still TBD? + * Shape of SyntaxNodes + * Shape of BoundNode \ No newline at end of file From f11460cc2952787da7a76c229f68997e9755b51e Mon Sep 17 00:00:00 2001 From: AdamSpeight2008 Date: Sun, 7 Jan 2018 16:32:42 +0000 Subject: [PATCH 2/5] Fix some spelling errors and add link to prototype. --- proposals/proposal-FlagsEnumOperators.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/proposals/proposal-FlagsEnumOperators.md b/proposals/proposal-FlagsEnumOperators.md index 5f5be11..66f5d50 100644 --- a/proposals/proposal-FlagsEnumOperators.md +++ b/proposals/proposal-FlagsEnumOperators.md @@ -1,7 +1,7 @@ # Feature: Flags Enum Operators * [x] Proposed -* [ ] Prototype: [Complete](https://github.com/PROTOTYPE_OWNER/roslyn/BRANCH_NAME) +* [ ] Prototype: **Very Experimental** [Proof of Concept](https://github.com/AdamSpeight2008/roslyn-AdamSpeight2008/tree/EnumFlagExpression) * [ ] Implementation: [In Progress](https://github.com/dotnet/roslyn/BRANCH_NAME) * [ ] Specification: [Not Started](pr/1) @@ -33,7 +33,7 @@ End Enum * **Clear** * `MyFlags = (MyFlags And Not Flags.Red)` -It would good if we could define a set of generic extension methods that implement those functions. +It would good be if we could define a set of generic extension methods that implement those functions. ```vbnet Imports System.Runtime.CompilerServices @@ -74,7 +74,7 @@ We could us a non generic implementation, but that incurs a runtime reflection c This is proposal introduces the concept of a **Flags Enum Operator**. A Flags Enum operator consist of three parts * `Flags` - * The flags enum (or variable of a flags enum) being work on. + * The flags enum *(or variable of a flags enum)* being work on. * `!` * The operation being performed. * 'flag` @@ -88,7 +88,7 @@ Initially one will be supported * This usage is analogous to `IsSet`, those returns a `Boolean`. * `IsClr` is supported via negation `Not flags!flag` or comparing against `flags!flag = false`. -Two other additional operator, are to be consider also;= +Two other additional operator, are to be consider also;- * `flags!+flag` * This is analogous to `[Set](Of Fe As Enum)(flags As Fe, Flag As Fe) As Fe` * `flags!-flag` @@ -126,7 +126,7 @@ Should be relatively simple to extend existing support for dictionary lookup ope Existing usage `Dim results = Flags!Flag` results in an error. `Error BC30103 '!' requires its left operand to have a type parameter, class or interface type, but this operand has the type 'Main.Flags` -So could extended under a feature flags, to preserve compatability. +Would be enable via a feature flag, to preserve compatability. The operator `!+` and `!-` will require additional parsing code to support. From b521be28c8aca19c7655b5c366c7cf385ff25482 Mon Sep 17 00:00:00 2001 From: AdamSpeight2008 Date: Sat, 13 Jan 2018 16:54:11 +0000 Subject: [PATCH 3/5] - Correct spelling - Add details of possible layouts for SyntaxNodes and BoundNodes. --- proposals/proposal-FlagsEnumOperators.md | 55 +++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/proposals/proposal-FlagsEnumOperators.md b/proposals/proposal-FlagsEnumOperators.md index 66f5d50..7667616 100644 --- a/proposals/proposal-FlagsEnumOperators.md +++ b/proposals/proposal-FlagsEnumOperators.md @@ -85,7 +85,7 @@ A Flags Enum operator consist of three parts Initially one will be supported * `flags!flag` * This borrows syntax from them dictionary lookup operator `!`. - * This usage is analogous to `IsSet`, those returns a `Boolean`. + * This usage is analogous to `IsSet`, which returns a `Boolean`. * `IsClr` is supported via negation `Not flags!flag` or comparing against `flags!flag = false`. Two other additional operator, are to be consider also;- @@ -130,6 +130,59 @@ Would be enable via a feature flag, to preserve compatability. The operator `!+` and `!-` will require additional parsing code to support. +### SyntaxNode +``` +Syntax FlagsEnumOperatorToken Inherits SyntaxToken + DefaultFactory = True + DefaultTrailingTrivia = None + + NodeKinds + "!+" -> FlagsEnumSetToken + "!-" -> FlagsEnumClearToken + "!" -> FlagsEnumIsSetToken + End NodeKinds + + +End Syntax + +Partial Syntax FlagsEnumOperationExpressionSyntax Inherits ExpressionSyntax + + NodeKinds + FlagsEnumOperationExpression + End NodeKinds + + Then expression on the left-hand-side of the flags enum operator. + .EnumFlags? As ExpressionSyntax + + + .OperatorToken As FlagsEnumOperatorTokrn + + The member of the flags enum, after the flags enum operator. + .EnumFlag As SimpleNameSyntax +End Syntax + +Enum FlagsEnumOperatorKind + .None = 0 + The "!" FlagsEnum Operator. + .IsSet = 1 + The "!+" FlagsEnum Operator. + .[Set] = 2 + The "!-" FlagsEnum Operator. + .Clear = 3 +End Enum +``` + +### BoundNode + +``` +Bound BoundFlagsEnumOperationExpressionSyntax Inherits ExpressionSyntax + Overrides Type As TypeSymbol (Null:= Disallow) + EnumFlags As BoundExpression (Null:= Disallow) + Op As FlagsEnumOperatorKind (Null:= NotApplicable) + EnumFlag As BoundExpression (Null:= Disallow) +End Bound +``` + ## Advantages * Simplified usage. From 81e52e2a7a8fda8f12e74265abaf43a27ab11a1d Mon Sep 17 00:00:00 2001 From: Adam Speight Date: Sun, 11 Feb 2018 09:19:40 +0000 Subject: [PATCH 4/5] Update proposal-FlagsEnumOperators.md --- proposals/proposal-FlagsEnumOperators.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proposals/proposal-FlagsEnumOperators.md b/proposals/proposal-FlagsEnumOperators.md index 7667616..308f964 100644 --- a/proposals/proposal-FlagsEnumOperators.md +++ b/proposals/proposal-FlagsEnumOperators.md @@ -1,14 +1,14 @@ # Feature: Flags Enum Operators * [x] Proposed -* [ ] Prototype: **Very Experimental** [Proof of Concept](https://github.com/AdamSpeight2008/roslyn-AdamSpeight2008/tree/EnumFlagExpression) +* [x] Prototype: Alpha [Proof of Concept](https://github.com/AdamSpeight2008/roslyn-AdamSpeight2008/tree/EnumFlagExpression) * [ ] Implementation: [In Progress](https://github.com/dotnet/roslyn/BRANCH_NAME) * [ ] Specification: [Not Started](pr/1) ## Summary [summary]: #summary -Simplifyy the usage of flags enums. +Simplify the usage of flags enums. ## Motivation [motivation]: #motivation @@ -203,4 +203,4 @@ What other designs have been considered? What is the impact of not doing this? What parts of the design are still TBD? * Shape of SyntaxNodes - * Shape of BoundNode \ No newline at end of file + * Shape of BoundNode From 5fb7dbf17a9ea31d0cbc42ec07281c29a9766e6a Mon Sep 17 00:00:00 2001 From: AdamSpeight2008 Date: Tue, 13 Feb 2018 19:45:28 +0000 Subject: [PATCH 5/5] Updating the proposal with more information. --- proposals/proposal-FlagsEnumOperators.md | 128 +++++++++++++++-------- 1 file changed, 86 insertions(+), 42 deletions(-) diff --git a/proposals/proposal-FlagsEnumOperators.md b/proposals/proposal-FlagsEnumOperators.md index 308f964..d127116 100644 --- a/proposals/proposal-FlagsEnumOperators.md +++ b/proposals/proposal-FlagsEnumOperators.md @@ -1,14 +1,21 @@ # Feature: Flags Enum Operators * [x] Proposed -* [x] Prototype: Alpha [Proof of Concept](https://github.com/AdamSpeight2008/roslyn-AdamSpeight2008/tree/EnumFlagExpression) +* [x] Prototype: [Proof of Concept](https://github.com/AdamSpeight2008/roslyn-AdamSpeight2008/tree/EnumFlagExpression + * [x] Syntax & Bound Nodes + * [x] Parser + * [x] Binding + * [x] Lowering + * [x] Error Messages *(Will need updating)* + * [x] Syntax Highlighting + * [ ] Constant Folding * [ ] Implementation: [In Progress](https://github.com/dotnet/roslyn/BRANCH_NAME) * [ ] Specification: [Not Started](pr/1) -## Summary +## Summary ^([VBLANG Issue#228](https://github.com/dotnet/vblang/issues/228))^ [summary]: #summary -Simplify the usage of flags enums. +Simplifyy the usage of flags enums, by providing a set of operators. ## Motivation [motivation]: #motivation @@ -65,10 +72,7 @@ End Module Unfortunatly this isn't currently possible in VB.net (as of VB15.5). We could us a non generic implementation, but that incurs a runtime reflection cost. - -## Detailed design -[design]: #detailed-design - + ### Overview of the operators This is proposal introduces the concept of a **Flags Enum Operator**. @@ -77,20 +81,22 @@ A Flags Enum operator consist of three parts * The flags enum *(or variable of a flags enum)* being work on. * `!` * The operation being performed. - * 'flag` + * 'Flag` * The flag to work with. * Note: `flag` is a literal referencing the member of the flags to use. - * Note: It isn't a reference to variable. + * Note: It isn't a reference to variable. *Maybe support in the future?* -Initially one will be supported - * `flags!flag` +##### `!` IsFlagSet Operator + * `Flags ! Flag` * This borrows syntax from them dictionary lookup operator `!`. * This usage is analogous to `IsSet`, which returns a `Boolean`. * `IsClr` is supported via negation `Not flags!flag` or comparing against `flags!flag = false`. -Two other additional operator, are to be consider also;- - * `flags!+flag` +##### `!+` SetFlag Operator + * `Flags !+ flag` * This is analogous to `[Set](Of Fe As Enum)(flags As Fe, Flag As Fe) As Fe` + +##### `!-` ClearFlag Operator * `flags!-flag` * This is analogous to `Clr(Of Fe As Enum)(flags As Fe, Flag) As Fe` * These two operators will have to special case in the compiler. @@ -108,18 +114,73 @@ Example code: ```VB.NET ' ... Dim myFlags = Flags.None -Dim flagsA = myFlags!+Red!+Green!+Blue -Dim flagsB = flagsA!-Green +Dim flagsA = myFlags !+ Red !+ Green !+ Blue +Dim flagsB = flagsA !- Green ' Explict comparision used for clarity. -If flagsA!Green = True Then +If flagsA ! Green = True Then ' ... -ElseIf flagsB!Green = True Then +ElseIf flagsB ! Green = True Then ' ... End If ' ... ``` -### Parsing +-------- + +## Advantages + + * Simplified usage. + * Compiler-Time verification of + * supported enums. + * Compile-Time Constant Folding. *As they are essentially constants.* + * Could also provide language support the proposed CoreFX Api Update ([CoreFX Enum Improvements](https://github.com/dotnet/corefx/issues/15453)) + * By changing the lowering. + * Akin to ValueTuples + +## Drawbacks +[drawbacks]: #drawbacks + +Why should we *not* do this? + +## Alternatives +[alternatives]: #alternatives + + * Extension Method + * Still Require a runtime reflection. + * Not restricted to just ` Enum` + * VB don't support *(as of this proposal)* attributes on generic type parameters + + * @AnthonyDGreen + * Non-Immutable. + * Looks to mutate the existing variable. + * Breaking change. + * >This could technically be a very minor breaking change because VB today lets you define an "extension readonly default property" on any type by defining an ElementAtOrDefault extension function and a Select extension method to make it a queryable type. There is no way to emulate the case of making the default property writeable. + >This entire scenario seems extremely unlikely but the breaking change could be avoided entirely by binding this way only after the queryable/ElementAtOrDefault check fails. + + * CoreFX Api Update ([CoreFX Enum Improvements](https://github.com/dotnet/corefx/issues/15453)) + * Requires a change at the core library level. + + +------------------------- + +### Implementation Details + +## Visual Basic Specification Changes + +`DictionaryAccessExpession ::= (Expr : Expression?) '!' (Key : IdentifierOrKeyword);` + ++ If `Expr` evaluates to an `Enum` + + **True:** If `Expr` has the `` attribute. + + **True:** Then parse `Key` as a member of that enumeration. eg `Red`, `Blue`, `Green`. + + If `Key` is valid member then + + If the `Key` is the default value (`None` in our example) mark the key as a warning. + As usage should result in the value not being changed. + + Otherwise this is valid representation + + Otherwise Mark the `key` as an error *`Not a member of enumeration`* + + **False:** Mark `expr` as an error. *`Enum doesn't have the attribute `* + + **False** Continue as a normal dictionary lookup expression. + +#### Parsing Should be relatively simple to extend existing support for dictionary lookup operator. @@ -128,9 +189,13 @@ Existing usage `Dim results = Flags!Flag` results in an error. Would be enable via a feature flag, to preserve compatability. -The operator `!+` and `!-` will require additional parsing code to support. +The operator `!+` and `!-` will require peeking at the next character. + * The parser already does this to see if is an identifier, for the dictionary lookup + * So adding a couple of additional check + * Is next character "+"c, parses as a potential FlagSet operator + * Is next character "-"c, parses as a potential FlagClear operator -### SyntaxNode +#### SyntaxNode ``` Syntax FlagsEnumOperatorToken Inherits SyntaxToken DefaultFactory = True @@ -172,7 +237,7 @@ Enum FlagsEnumOperatorKind End Enum ``` -### BoundNode +#### BoundNode ``` Bound BoundFlagsEnumOperationExpressionSyntax Inherits ExpressionSyntax @@ -183,24 +248,3 @@ Bound BoundFlagsEnumOperationExpressionSyntax Inherits ExpressionSyntax End Bound ``` -## Advantages - - * Simplified usage. - * Compiler-Time verification of supported enums. *As they are essentially constants.* - -## Drawbacks -[drawbacks]: #drawbacks - -Why should we *not* do this? - -## Alternatives -[alternatives]: #alternatives - -What other designs have been considered? What is the impact of not doing this? - -## Unresolved questions -[unresolved]: #unresolved-questions - -What parts of the design are still TBD? - * Shape of SyntaxNodes - * Shape of BoundNode