search

vrchat-community / UdonSharp

A compiler for compiling C# to Udon assembly
https://udonsharp.docs.vrchat.com
MIT License
470 stars 50 forks source link

Don't COW the internal `this` constant #10

Closed bdunderscore closed 2 years ago

bdunderscore commented 2 years ago

Certain ternary expressions could result in COWing the internal this constant unnecessarily. Mark this as constant to avoid this.

Because this hides another bug, I've added a (currently failing) test that detects the more general case of inappropriate COWing across ternary operators. I've not added it to the scene because I'm sure the merge will be a nightmare (and it's failing until some of Merlin's working branch changes land)

This is a resubmission of https://github.com/MerlinVR/UdonSharp/pull/132 against the newer branch and new repo.

github-actions[bot] commented 2 years ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

bdunderscore commented 2 years ago

I have read the CLA Document and I hereby sign the CLA

MerlinVR commented 2 years ago

The issue with COWs getting dirtied conditionally is fixed for the moment by dirtying all COW values before entering flow control: https://github.com/vrchat-community/UdonSharp/blob/75bbcc8179ac5592fcbb0316f25c0055534b1bd0/Assets/UdonSharp/Editor/Compiler/Binder/BoundNodes/BoundConditionalExpression.cs#L38 the fix may be revisited at some point to have more fine-grained ValueTable scopes, but will need to allow inline declarations.