Skip to content

Add 'bitflags' modifier for enum. #42533

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.

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

Closed

Conversation

ShuiRuTian
Copy link
Contributor

@ShuiRuTian ShuiRuTian commented Jan 28, 2021

Fixes #42521

  • Add modifier bitflags.
  • bit operator(|,|=,&,&=,^,^=) is only allowed for bitflags enum, if left or right is enum.
  • Add new flag to enable bitflags check.

The second one is a big break! Maybe it should be loosen, but how?

And I think there are more features could be added!

  • bitflags enum member could only be initialized by BitwiseOperator(&,|,^) or ShiftOperator(<<,>>,>>>) or bit number
bitflags enum BitEnum {
    TS = 1,     // 1 is 1 << 0, OK
    TSX = 2,    // 2 is 1 << 1, OK
    ERR2 = "1", // "1" is not number, error
    All = TS | TSX, // initialized through `|`, should be OK without "1"
    All2 = 3,    // 3 is not a bit, error
    All3 = 1+2 // '+' is not  BitwiseOperator or ShiftOperator, error.
}
  • more accurate type.
bitflags enum BitEnum {
    TS = 1,
    TSX = 2,
    TSAlias = TS,
    All = TS | TSX,
}
const foo1 = BitEnum.TSAlias;
const foo2 = BitEnum.TSX;
const foo3 = foo1 | foo2;    // what is foo1 type? `BitEnum.TS` or `BitEnum.TSAlias`?
  • give accurate type when both bitflags enum are known.
bitflags enum BitEnum {
    TS = 1,
    TSX = 2,
    All = TS | TSX,
}

var foo1 = BitEnum.TS;
var foo2 = BitEnum.TSX;
var foo3 = foo1 | foo2; // foo3 is possiable to be BitEnum.All?
declare var foo4: BitEnum;
var foo5 = foo4 & foo1; // foo5 is possiable to be BitEnum.TS?

Some Questions:

  1. What should be the flag name? Should it be "strict"? Personally, I want to enable this feature in any new project, but it might be a big break for current project. Now it is bitEnum
  2. Is there a better name rather than bitflags?
  3. How to mark enum as bit? Should bitflags be a modifier or tag or something else?
  4. Are error messages easy to understand? I am clearly not a English native speaker, they might looks wired now.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jan 28, 2021
@weswigham
Copy link
Member

@ShuiRuTian we were discussing allowing the modifier/tag/whatever in any mode, but only having it do anything different from normal enums today under a new flag - so you can opt in to strictly checking bitflagginess. This way it wouldn't be a straight break for anyone currently using normal enums as bitflags.

@@ -9574,29 +9576,20 @@ namespace ts {
if (links.declaredType) {
return links.declaredType;
}
if (getEnumKind(symbol) === EnumKind.Literal) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is EnumKind on earth?

@ShuiRuTian
Copy link
Contributor Author

ShuiRuTian commented Jan 29, 2021

@weswigham Thanks a lot! the beta version is ready for review.

But, I find something strange about the enums, which blocks me on more accurate type. item in the check list.

It is basically about EnumKind. I feel EnumKind.Numeric is something like number, and EnumKind.Literal is like string.

But at the same time, we could see in function getDeclaredTypeOfEnum, in the block we have also getLiteralType and getRegularTypeOfLiteralType. Literal for these two functions is more like Syntax.stringLiteral and Syntax.numericLiteral.

Also, it seems if type is EnumKind.Numeric, then enum members does not have any chance to have a correct declaredType -- if it is not initialized, in the second time it could only get its parent cached type(Not sure about this, but I am sure enum member in EnumKind.Numeric does not have as rich flags as those members in EnumKind.Literal).


PS: I change a big else-if to

switch(true){
case (condition):
...
}

Reason: this helps debug, the code would jump to the first true condition, pass a lot false condition.
But is this good or at least acceptable? Is this too tricky to make code hard to read?
Would revert them if you think this trade-off is not good.

@ShuiRuTian ShuiRuTian marked this pull request as ready for review January 29, 2021 11:06
@typescript-bot
Copy link
Collaborator

The TypeScript team hasn't accepted the linked issue #42521. If you can get it accepted, this PR will have a better chance of being reviewed.

@weswigham
Copy link
Member

weswigham commented Jan 29, 2021

I feel EnumKind.Numeric is something like number, and EnumKind.Literal is like string

Numeric is the old bitflag allowing enum, while Literal is the newer union-of-uniqe-literal-subtypes enum. You get the later if the enum contains a string initializer today. You probably wanna make it so when the strict bitflag flag is set, all enums are Literal unless they have the bitflag modifier, at which point they're Numeric-ish.

@ShuiRuTian

This comment has been minimized.

@ShuiRuTian ShuiRuTian marked this pull request as draft February 2, 2021 03:09
@ShuiRuTian
Copy link
Contributor Author

Seems a new keyword is not acceptable. Would restart working on this if there is any new thought.

@sandersn
Copy link
Member

To help with PR housekeeping, I'm going to close this draft PR. It is pretty old now.

@sandersn sandersn closed this May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modifier for bitflag enums
4 participants