Skip to content

SyntheticBoundNodeFactory: Remove ability to create arbitrary conversions #81569

Merged
AlekseyTs merged 1 commit intodotnet:mainfrom
AlekseyTs:SyntheticBoundNodeFactory_Convert
Dec 8, 2025
Merged

SyntheticBoundNodeFactory: Remove ability to create arbitrary conversions #81569
AlekseyTs merged 1 commit intodotnet:mainfrom
AlekseyTs:SyntheticBoundNodeFactory_Convert

Conversation

@AlekseyTs
Copy link
Contributor

Language rules around conversions are getting more and more complex. At the same time Convert APIs in SyntheticBoundNodeFactory might confuse consumer into thinking that they are able to synthesize nodes for arbitrary conversions, which is not the case.

This change removes one API and adds an assert to remaining API to verify that only conversions that are natively supported by Emit layer are coming through.

…eFactory

Language rules around conversions are getting more and more complex.
At the same time Convert APIs in SyntheticBoundNodeFactory might confuse
consumer into thinking that they are able to synthesize nodes for arbitrary conversions,
which is not the case.

This change removes one API and adds an assert to remaining API to verify that
only conversions that are natively supported by Emit layer are coming through.
@AlekseyTs AlekseyTs requested a review from a team as a code owner December 6, 2025 01:25
Comment on lines +193 to +194
case ConversionKind.Boxing:
case ConversionKind.ExplicitReference:
Copy link
Member

@333fred 333fred Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit:

Suggested change
case ConversionKind.Boxing:
case ConversionKind.ExplicitReference:
case ConversionKind.ExplicitReference:
case ConversionKind.Boxing:
``` #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order of cases came from the function above

@AlekseyTs AlekseyTs requested review from a team and RikkiGibson December 6, 2025 03:21
@AlekseyTs
Copy link
Contributor Author

@RikkiGibson, @dotnet/roslyn-compiler For a second review

@jcouv jcouv self-assigned this Dec 8, 2025
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Thanks (commit 1)

@AlekseyTs AlekseyTs merged commit 8102dd3 into dotnet:main Dec 8, 2025
26 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Dec 8, 2025
333fred added a commit to 333fred/roslyn that referenced this pull request Dec 9, 2025
* upstream/main: (221 commits)
  [main] Update dependencies from dotnet/roslyn (dotnet#81601)
  Update Roslyn.Diagnostics.Analyzers (dotnet#81599)
  Remove unnecessary lightup code
  Remove unnecessary lightup code
  Remove unnecessary lightup code
  Remove unnecessary lightup code
  Remove unnecessary lightup code
  Build
  NullableWalker.DebugVerifier: Explicitly visit `BoundCollectionExpressionSpreadElement.Conversion` (dotnet#81584)
  SyntheticBoundNodeFactory: Remove ability to create arbitrary conversions  (dotnet#81569)
  Docs
  Tweak
  Remove dependency on Newtonsoft.Json from Microsoft.CodeAnalysis.Workspaces.MSBuild.BuildHost (dotnet#81562)
  Simplify
  Apply suggestions from code review
  Make into theory
  Updates
  Add tests
  Add tests
  Improve go-to-def navigation in overload resolution failure cases
  ...
@davidwengier davidwengier modified the milestones: Next, 18.3 Jan 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants