Include 'const' keyword range in static constant expr range #10165
Conversation
src/fsharp/SyntaxTree.fs
Outdated
| @@ -507,6 +507,7 @@ type SynType = | |||
| /// F# syntax: const expr, used in static parameters to type providers | |||
| | StaticConstantExpr of | |||
| expr: SynExpr * | |||
| exprRange: range * | |||
There was a problem hiding this comment.
Is there a need for this field? Let's calculate this range from expr, like it's done in SynExpr.Do, SynExpr.Lazy, and SynExpr.Assert?
DedSec256
force-pushed
the
ber.a/staticConstExprRange
branch
from
1f33cb4 to
62c9f5e
Compare
There was a problem hiding this comment.
Looks good to me.
Your previous change with the exprRange to me feels more correct because if we ever need to get the range that doesn't include const, we would be able to. But, it would be inconsistent with other SynExprs based on what @auduchinok mentioned. Therefore, I think the current change is fine.
|
@TIHan it's still possible to get the expression range, it'll be calculated from the expression itself instead of being stored in its parent node. Since it's not something used too often it looks like a good trade off and apparently it hasn't been an issue with the other cases mentioned above either. |
|
@auduchinok That works better actually; I didn't initially think about that. |
For static constant expressions like the following, add an extra range for the entire StaticConstantExpr including 'const'.
This will make it easier to understand the boundaries of the entire static constant expression