Decimal constant improvements #1236
Conversation
ashmind
changed the title
Added option to uglify presentation of decimal constants (show `[DecimalConstant(...)]`).
ashmind
force-pushed
the
feature/decimal-constant-improvements
branch
from
2999383
to
d15fe0f
Compare
There was a problem hiding this comment.
I didn't realize there was a difference between these methods, and that C# can call both without specifying an argument:
public static void Test1(CancellationToken ct = default) { }
public static void Test2([Optional] CancellationToken ct) { }Given this difference, the introduction of HasConstantValueInSignature makes sense.
The PR looks good to me (except for the thread-safety issues). Thanks!
| @@ -42,7 +42,7 @@ sealed class MetadataField : IField | |||
| object constantValue; | |||
| IType type; | |||
| bool isVolatile; // initialized together with this.type | |||
| byte decimalConstant; // 0=no, 1=yes, 2=unknown | |||
| bool? isDecimalConstant; | |||
There was a problem hiding this comment.
This change is not thread-safe.
In the rare case where multiple threads are concurrently initializing isDecimalConstant, we need accesses to be atomic.
But Nullable<T> is a struct and may have torn reads/writes.
| @@ -37,6 +37,8 @@ sealed class MetadataParameter : IParameter | |||
|
|
|||
| // lazy-loaded: | |||
| string name; | |||
| bool? hasConstantValueInSignature; | |||
| bool? isDecimalConstant; | |||
There was a problem hiding this comment.
Same thread-safety issues here.
|
@dgrunwald Thanks, fixed -- didn't ever consider torn reads before, something to keep in mind. |
…tant-improvements
This covers two main things:
Added support for decimal optional parameters, e.g.
M(decimal d = 1m). Since those are stored as[DecimalConstant(...)]and not standard constants, previously they would not decompile correctly.Added support for new setting
DecimalConstants. If set tofalse, decimal constants and optional parameters will explicitly expose[DecimalConstant(...)]instead of pretending to be a literal. This is required by SharpLab since I want to expose inner workings as close as possible.Both changes include unit-tests.
(ref: ashmind/SharpLab#316)