Emit C# 7.1 default literals where the context supplies the type - #4014
Open
christophwille wants to merge 3 commits into
Open
Emit C# 7.1 default literals where the context supplies the type#4014christophwille wants to merge 3 commits into
christophwille wants to merge 3 commits into
Conversation
Shortening default(T) is the same problem as removing the redundant cast around a lambda whose delegate type the context already fixes, so it uses the same mechanism: ConvertTo makes the explicit type implicit when the conversion is an identity conversion and the caller allows an implicit one. The literal keeps the type it was shortened from, so any later conversion to a different type - or any context that requires an explicit type, such as an overload resolution recheck falling back to CastArguments - can spell default(T) out again. That keeps the value intact where the bare literal would change it, e.g. "object o = default(SomeStruct)", which boxes a non-null struct while "default" would be null. Because the shortened literal resolves to DefaultLiteralResolveResult, CallBuilder's existing overload resolution recheck sees a real default literal and rejects ambiguous calls on its own; no separate bookkeeping about which arguments may stay untyped is needed. Only the contexts that supply no target type at all restore the explicit form: an awaited expression, and arguments of operator methods, which later become operator or cast syntax rather than calls. Assisted-by: Claude:claude-opus-5[1m]:Claude Code
dgrunwald
reviewed
Aug 16, 2026
| { | ||
| // The target type is supplied by the context, so "default(T)" can be | ||
| // shortened to the C# 7.1 default literal. | ||
| var shortened = new DefaultValueExpression(); |
Member
There was a problem hiding this comment.
Cloning the node is unnecessary, this could have been an in-place modification.
Reviewed with Stampeded!
| { | ||
| value = value.ConvertTo(expectedType, this, allowImplicitConversion: true); | ||
| // The awaited expression is not target-typed: "await default" does not compile. | ||
| value = value.RestoreDefaultLiteralType(this); |
Member
There was a problem hiding this comment.
This is a workaround for a bug we'll fix separately on another branch -- really await shouldn't be using allowImplicitConversion
.
Mutating the DefaultValueExpression is enough here; ConvertTo already hands out mutated input nodes elsewhere (UnwrapChild), so building a replacement node and copying the annotations over bought nothing. The operator special case is easy to mistake for a cosmetic preference, because the null literal is accepted in the same position: it converts only to reference and nullable types, so it still narrows operator overload resolution, whereas the default literal converts to everything and C# rejects it outright for every binary operator except == and !=. Assisted-by: Claude:claude-opus-5[1m]:Claude Code
Making a conversion implicit by unwrapping it hands the operand to a different target type, and a default literal takes its value from that type: "S? x = new S?(default)" holds a value, while "S? x = default" is null. Unwrapping the nullable constructor around a shortened literal therefore turned "S? x = default(S)" into a null nullable. The literal is spelled out again whenever unwrapping moves it to a type other than the one it was shortened from. Converting a using resource to the declared variable type is unconditional now (except when the declaration says "var", which supplies no type): the declaration always spells the type out, so any conversion to it may stay implicit, which is also what shortens default(T) there. Assisted-by: Claude:claude-opus-5[1m]:Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Alternative implementation of #3913: same feature, same tests, different mechanism.
Approach
Shortening
default(T)is the same problem as removing the redundant cast around a lambda whose delegate type the context already fixes, so it uses the same mechanism instead of a dedicated AST transform:default(T)is emitted as before, andTranslatedExpression.ConvertTo(targetType, allowImplicitConversion: true)makes the explicit type implicit when the conversion is an identity conversion - a few lines above the existing lambda/ConversionResolveResultcase in the very same block.DefaultValueExpressionloses its type and is annotated withDefaultLiteralResolveResult, which remembers the type it was shortened from.ConvertTothat is not implicit, or that supplies a different type, spellsdefault(T)out again and converts that. This is what keepsobject o = default(SomeStruct)boxing (the bare literal would benull), and whatCastArgumentsuses when an overload resolution recheck rejects the untyped literal.DefaultLiteralResolveResult,CallBuilder's existingIsUnambiguousCallrecheck sees a real default literal and rejects ambiguous calls on its own - no bookkeeping about which arguments may stay untyped, no argument annotations.Only the contexts that supply no target type at all restore the explicit form via
RestoreDefaultLiteralType: an awaited expression (await defaultdoes not compile) and arguments of operator methods (ReplaceMethodCallsWithOperatorsturns those into operator or cast syntax). Two further spots opt in to the shortening because their declaration always spells out the type:DeclareVariables' synthesized initializer andusingresource declarations.Compared to #3913 this drops the
IntroduceDefaultLiteralstransform,UseImplicitlyTypedDefaultAnnotationand theArgumentList.UseImplicitlyTypedDefaultplumbing inCallBuilder. Kept from it unchanged: optionalDefaultValueExpression.Typeplus output visitor,DefaultLiteralResolveResult/Conversion.DefaultLiteralConversion/ theCSharpConversionshook, and theDefaultLiteralssetting (C# 7.1, on by default).Tests
The tests are the ones from #3913. Three expectations differ, because
ConvertTocovers different ground than the transform did - each fixture is also the compiled input, so csc validates every one of them:this[default]- indexer arguments shorten (Emit C# 7.1 default literals #3913 pinned them typed); the fixture method is renamedIndexerArgumentStaysTyped->IndexerArgument.() => defaultinGuidExpressionTree- expression-tree lambda bodies shorten.Params(default(int), default(int), default(int))- params-array elements synthesized by the params expansion never pass throughConvertTo, so they keep their type.These fixtures needed updating on top of #3913's set, because
ConvertToreaches contexts the transform did not:RefFields,RefStructInterfaces,FirstClassSpanTypes,FirstClassSpanConversions,QueryExpressions,DeconstructionTests,SpanConversionOperatorMismatch. InIssue2260SwitchStringthedecimal x = default;lines from #3913 are back to0m, which is what master emits there today.Full
ICSharpCode.Decompiler.Testssuite passes (the Windows-only test projects were not run - this was developed on macOS).