Skip to content

Support the tractable C# patterns when converting to VB - #1282

Open
gherards99 wants to merge 1 commit into
icsharpcode:masterfrom
gherards99:vb-pattern-matching
Open

Support the tractable C# patterns when converting to VB#1282
gherards99 wants to merge 1 commit into
icsharpcode:masterfrom
gherards99:vb-pattern-matching

Conversation

@gherards99

Copy link
Copy Markdown

Fixes #1281. Fixes #1112. Partially addresses #983.

Problem

VisitIsPatternExpression handled only DeclarationPatternSyntax and ConstantPatternSyntax and threw ArgumentOutOfRangeException for anything else, so type patterns, negated patterns and relational patterns all failed to convert — details and repro in #1281. Separately, a constant pattern compared by value was emitted with Is, which compares references in VB, so the output did not compile and nothing reported it.

Solution

  • VisitIsPatternExpression now delegates to a recursive ConvertPattern, which is what lets a negated pattern convert the pattern it wraps. Type patterns become TypeOf x Is T, relational patterns become plain comparisons, and negation is flipped in place — TypeOf x IsNot T, or TryCast(x, T) Is Nothing — rather than wrapping the condition in Not. That matches how the existing declaration-pattern case is written and reads better in the output.

  • A constant pattern whose expression binds to a type is a type test, so it emits TypeOf x Is T. Compared by value it emits = for a value type and Equals for anything else, because on an Object the VB = goes through late binding and throws where C# answers False. Equals(...) is already how the surrounding code compares strings, so this follows existing practice rather than introducing a new idiom.

  • x is not T v declares v just as x is T v does, so the hoisting in CommonConversions follows the negation to find it.

  • ImplicitObjectCreationExpression had no visitor, which is C# -> VB: Target typed new does not convert #1112; the type the compiler settled on is written out.

  • GetSymbolInfo(node.Left).Symbol was dereferenced without a null check while converting an event subscription, so one unresolved symbol failed the whole file instead of degrading.

  • A failed expression conversion comes back as an empty statement carrying its report as trivia, and a return discarded both, losing the value and the diagnostic together. The report now travels with the statement. That is the symptom in C# -> VB: Switch Expression in Return Statement Converts to an Empty String #983 — a switch expression in a return produced an empty return with nothing said. This does not implement switch expressions, it makes the failure visible.

Which part is most in need of attention: the constant-pattern rule sends string through Equals as well, which is correct but less idiomatic than =. Narrowing it to unresolved and Object operands would keep = for strings — I would rather you decide which you prefer.

Deliberately left out: property patterns and ??=. Both need the tested expression evaluated once rather than repeated, which means introducing a temporary and touching the hoisting machinery the contributing guide flags as delicate. Happy to raise those separately.

Process

  • Attachment level: medium. I understand each change and verified every claim by reproducing it against a clean master and running the suite. I would defer to you on the Equals question above, and on whether the return trivia change belongs here or in its own PR.

  • LLM primarily used: Claude Opus 5.

  • On prompts: this came out of a long working session rather than a single prompt, so there is no one prompt worth pasting. The method was: reproduce each failure on master with a minimal snippet, read the visitor to find where it throws, implement the case, then re-convert the snippet and run the suite.

VisitIsPatternExpression handled only declaration and constant patterns and
threw for everything else, so most pattern matching failed to convert. It is
now split into a recursive ConvertPattern that also covers:

* type patterns, as TypeOf x Is T
* negated patterns, flipped in place to TypeOf x IsNot T or TryCast(x, T) Is
  Nothing rather than wrapped in Not
* relational patterns, as plain comparisons

A constant pattern whose expression binds to a type is a type test, not a
comparison, and was being emitted as Is, which compares references in VB. It
now emits TypeOf x Is T. A constant compared by value went the same way and did
not compile at all; it now uses = for a value type and Equals for anything
else, which is what the pattern means when the operand is an Object.

"x is not T v" declares v just as "x is T v" does, so the hoisting in
CommonConversions follows the negation to find it.

Also here, both smaller and separable:

* ImplicitObjectCreationExpression had no visitor, so target-typed new threw.
  The type the compiler settled on is written out instead.
* GetSymbolInfo(node.Left).Symbol was dereferenced without a null check while
  converting an event subscription, so an unresolved symbol crashed the whole
  file rather than degrading.
* A failed expression conversion comes back as an empty statement carrying its
  report as trivia. A return statement dropped both, losing the value and the
  diagnostic together; the report now travels with the statement.

Property patterns and ??= are deliberately left out: both need the tested
expression evaluated once, which needs a temporary rather than repeating it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

C# -> VB: most pattern matching fails to convert C# -> VB: Target typed new does not convert

1 participant