Autonomous problem fix#1
Conversation
| @@ -1,4 +1,4 @@ | |||
| using System; | |||
| using System; | |||
There was a problem hiding this comment.
This is noise in the delta. BOM must not be added or removed.
| { | ||
| var s = Unsafe.As<string>(arg); | ||
| int padding = width - s.Length; | ||
| int padding = width - s!.Length; |
There was a problem hiding this comment.
The change is correct, but should be applied at the point where s is initialized, like this - var s = Unsafe.As<string>(arg)!;. That way there would be no need to modify all the usages of s.
There was a problem hiding this comment.
The compiler is smart enough to propagate the nullness property of the variable after the null forgiveness operator is applied to it. It is also smart enough to figure out that the null check should happen on the first use of the variable along an execution path.
byte[]? s = null;
s[0] = 1; // Warning here.
s[1] = 1; // No warning here.byte[]? s = null;
s![0] = 1;
s[1] = 1;But it seems that model is unaware of this fact, and liters the code with null forgiveness operator.
| var span = sb.GetSpan(s.Length); | ||
| s.AsSpan().CopyTo(span); | ||
| sb.Advance(s.Length); | ||
| var span = sb.GetSpan(s!.Length); |
There was a problem hiding this comment.
Redundant when the initialization of s is adjusted - var s = Unsafe.As<string>(arg)!;
| s.AsSpan().CopyTo(span); | ||
| sb.Advance(s.Length); | ||
| var span = sb.GetSpan(s!.Length); | ||
| s!.AsSpan().CopyTo(span); |
There was a problem hiding this comment.
Redundant when the initialization of s is adjusted - var s = Unsafe.As<string>(arg)!;
| sb.Advance(s.Length); | ||
| var span = sb.GetSpan(s!.Length); | ||
| s!.AsSpan().CopyTo(span); | ||
| sb.Advance(s!.Length); |
There was a problem hiding this comment.
Redundant when the initialization of s is adjusted - var s = Unsafe.As<string>(arg)!;
|
|
||
| public void TryGrow(int sizeHint) | ||
| { | ||
|
|
There was a problem hiding this comment.
Noise in the delta. Unsolicited code formatting change - empty lines must be preserved.
| { | ||
| var s = Unsafe.As<string>(arg); | ||
| int padding = width - s.Length; | ||
| int padding = width - s!.Length; |
There was a problem hiding this comment.
Redundant when the initialization of s is adjusted - var s = Unsafe.As<string>(arg)!;
| } | ||
|
|
||
| Append(s); | ||
| Append(s!); |
There was a problem hiding this comment.
Redundant when the initialization of s is adjusted - var s = Unsafe.As<string>(arg)!;
| if (formatter == null) | ||
| { | ||
| if (typeof(T).IsEnum) | ||
| if (typeof(T).IsEnum && typeof(T).IsValueType) |
There was a problem hiding this comment.
An enum type is already a value type. This change is redundant.
There was a problem hiding this comment.
There are numerous changes in this file that change buffer to buffer!. All of them are correct, but redundant if buffer declaration is adjusted from char[]? buffer; to char[] buffer;
There was a problem hiding this comment.
While true, the domain of the buffer field is indeed byte[]?. See the Dispose method which sets buffer to null and checks for it being null. I suppose ideally the methods would check if the struct was disposed, but that has performance implications.
Structs are also zero-init by default. For example default(Utf16ValueStringBuilder).AppendLine(); will NRE because buffer is null.
I argue that it should be a nullable type since it captures the true domain of the variable, and that explicitly adding the null forgiveness operator at use sites captures the intent of ignoring this nullability. Of course deciding whether buffer should be a nullable type is at the discretion of the author/maintainer.
- Added `where T : notnull` constraint to EnumUtil<T> to resolve CS8714 errors regarding nullability mismatch on generic type parameters. - Used null-forgiveness operator (`!`) where needed to resolve possible null reference warnings (CS8602, CS8604). - Ensured no behavior change, only nullability and constraint fixes.
- Added null-forgiveness operator (`!`) when using Unsafe.As<string>(arg) to resolve CS8602 warnings about possible null reference dereference. - Ensured all string conversions and usages are null-forgiven where the logic expects non-null values. - No behavior change, only null-safety improvements.
- Updated method signatures to match nullability of overridden members, e.g., `Write(string? value)` and `WriteAsync(string? value)`, to resolve CS8765 warnings. - Used null-forgiveness operator where necessary. - No behavior change, only null-safety and signature alignment.
- Added null-forgiveness operator (`!`) to buffer and string usages where the logic guarantees non-nullness, resolving CS8602 warnings. - No behavior change, only null-safety improvements.
- Added null-forgiveness operator (`!`) to buffer and string usages where the logic guarantees non-nullness, resolving CS8602 and CS8604 warnings. - No behavior change, only null-safety improvements.
- Added null-forgiveness operator (`!`) to `Unsafe.As<string>(values[0])` in `JoinInternal<T>(ReadOnlySpan<char>, ReadOnlySpan<T>)` to resolve CS8603 warning about possible null reference return. - No behavior change, only null-safety improvement.
2 participants
No description provided.