Const enums #970
Const enums #970
Conversation
| case SyntaxKind.AmpersandToken: return left & right; | ||
| case SyntaxKind.PlusToken: return left + right; | ||
| case SyntaxKind.MinusToken: return left - right; | ||
| case SyntaxKind.GreaterThanGreaterThanToken: return left >> right; |
There was a problem hiding this comment.
Should support >>> as well probably
There was a problem hiding this comment.
Why not include *, /, %, and ^ as well? Seems hard to define a principle that would exclude them.
There was a problem hiding this comment.
👍 to Anders' suggestion. Might be arguable as to what you want to do when the RHS for / and % are 0 though.
|
👍 |
|
Is there a particular reason these optimizations need to be under a flag. Could we just always do them? Also, an even more impactful optimization (that would need to be under a flag) is to completely erase enums from the generated code, i.e. require that all declared values can be evaluated to constants at compile time and that there are no references to the enum object itself anywhere in the code such that we can omit it in the generated code. |
|
@ahejlsberg We could always do them. We'd simply have to add that to the spec. Right now these are an extension to the language, and so they should not be happening without the user opting-in. Also, note that the complete erasure would also be something we'd need to spec, (and likely make into a switch that someone could opt-in/out of). Even if there are no references in the code at runtime, the enum may still be used during debugging, and we'd want some way for it to be there. |
|
I've tried to convert enums in core compiler to const enums. Size of tsc.js before conversion: ~930k, after conversion 887k |
| @@ -5158,8 +5164,8 @@ module ts { | |||
| } | |||
|
|
|||
| function inferTypeArguments(signature: Signature, args: Expression[], excludeArgument?: boolean[]): InferenceContext { | |||
| var typeParameters = signature.typeParameters; | |||
There was a problem hiding this comment.
merge artefact, this part was not changed
|
It is on my TODO list |
| isRhsOfImportStatement(node); | ||
|
|
||
| if (!ok) { | ||
| error(node, Diagnostics.const_enums_can_only_be_used_in_property_access_expressions); |
There was a problem hiding this comment.
Error message is updated in the latest bits and will be checked in shortly
|
2d94030 is technically a breaking change since we didn't used to inline indexed access expressions for regular enums. However spec defined property access by using either dot or bracket notations so I'd say current version is more consistent and aligned with a spec - property access (in any notation) for enum members is always inlined if member was initialized with a constant value (for regular enums) or with constant expression (for const enums) |
|
The only piece missing for me is the ability to have a command line switch to emit the reverse-enum-map. We definitely need that, or else debugging will be excessively painful. |
|
actually this functionality is already checked in - you need to specify 'preserveConstEnums' command line argument |
…tion to 'preserveConstEnums' command line argument
No description provided.