From b42e629809cb3cae9ae5eb9210690bfbe36ebae1 Mon Sep 17 00:00:00 2001 From: bd_ Date: Sun, 28 Nov 2021 17:34:36 -0800 Subject: [PATCH] Add support for searching for alternate invocees for externs For certain C# methods, Udon exposes the method via a different type than Roslyn considers it to be defined on. One example is `System.Type.Name`, which is actually defined in `System.Reflection.MemberInfo`, but exposed via `System.Type.Name`. This change allows U# to locate these alternate extern symbols and use them transparently. We move the logic for checking whether symbols are exposed to udon down into BoundExternMethodInvocation, then teach this class to search declared interfaces and superclasses of the _invocation_ target (rather than the symbol's containing type) for defined externs. Note that we assume that any exposed extern will match if we find it via a constructed symbol here. --- .../BoundArrayCreationExpression.cs | 2 +- .../BoundNodes/BoundExternMethodInvocation.cs | 54 +++++++- .../BoundNodes/BoundInvocationExpression.cs | 37 ++---- .../AccessViaAlternateInvocee.asset | 120 ++++++++++++++++++ .../AccessViaAlternateInvocee.asset.meta | 8 ++ .../AccessViaAlternateInvocee.cs | 26 ++++ .../AccessViaAlternateInvocee.cs.meta | 11 ++ 7 files changed, 232 insertions(+), 26 deletions(-) create mode 100644 Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.asset create mode 100644 Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.asset.meta create mode 100644 Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.cs create mode 100644 Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.cs.meta diff --git a/Assets/UdonSharp/Editor/Compiler/Binder/BoundNodes/BoundArrayCreationExpression.cs b/Assets/UdonSharp/Editor/Compiler/Binder/BoundNodes/BoundArrayCreationExpression.cs index cd37b2e2..01e07f21 100644 --- a/Assets/UdonSharp/Editor/Compiler/Binder/BoundNodes/BoundArrayCreationExpression.cs +++ b/Assets/UdonSharp/Editor/Compiler/Binder/BoundNodes/BoundArrayCreationExpression.cs @@ -27,7 +27,7 @@ public ArrayConstructorExtern(AbstractPhaseContext context, TypeSymbol arrayType private BoundExpression[] Initializers { get; } public BoundArrayCreationExpression(SyntaxNode node, AbstractPhaseContext context, TypeSymbol arrayType, BoundExpression[] rankSizes, BoundExpression[] initializers) - : base(node, new ArrayConstructorExtern(context, arrayType), null, rankSizes) + : base(node, context, new ArrayConstructorExtern(context, arrayType), null, rankSizes) { ArrayType = arrayType; Initializers = initializers; diff --git a/Assets/UdonSharp/Editor/Compiler/Binder/BoundNodes/BoundExternMethodInvocation.cs b/Assets/UdonSharp/Editor/Compiler/Binder/BoundNodes/BoundExternMethodInvocation.cs index 5a7ddc93..02db623d 100644 --- a/Assets/UdonSharp/Editor/Compiler/Binder/BoundNodes/BoundExternMethodInvocation.cs +++ b/Assets/UdonSharp/Editor/Compiler/Binder/BoundNodes/BoundExternMethodInvocation.cs @@ -1,17 +1,67 @@  using System; +using System.Collections.Generic; using System.Linq; using Microsoft.CodeAnalysis; using UdonSharp.Compiler.Emit; using UdonSharp.Compiler.Symbols; +using UdonSharp.Compiler.Udon; +using UdonSharp.Core; +using UdonSharp.Localization; namespace UdonSharp.Compiler.Binder { internal class BoundExternInvocation : BoundInvocationExpression { - public BoundExternInvocation(SyntaxNode node, MethodSymbol method, BoundExpression instanceExpression, BoundExpression[] parameterExpressions) + private ExternMethodSymbol externMethodSymbol; + + public BoundExternInvocation(SyntaxNode node, AbstractPhaseContext context, MethodSymbol method, BoundExpression instanceExpression, BoundExpression[] parameterExpressions) : base(node, method, instanceExpression, parameterExpressions) { + externMethodSymbol = (ExternMethodSymbol)method; + if (!CompilerUdonInterface.IsExposedToUdon(externMethodSymbol.ExternSignature)) + { + externMethodSymbol = FindAlternateInvocation(context, method, instanceExpression, parameterExpressions); + if (externMethodSymbol == null) + { + throw new NotExposedException(LocStr.CE_UdonMethodNotExposed, node, $"{method.RoslynSymbol?.ToDisplayString() ?? method.ToString()}, sig: {((ExternMethodSymbol)method).ExternSignature}"); + } + } + } + + private ExternMethodSymbol FindAlternateInvocation(AbstractPhaseContext context, MethodSymbol originalSymbol, BoundExpression instanceExpression, BoundExpression[] parameterExpressions) + { + if (originalSymbol.IsStatic || originalSymbol.IsConstructor) return null; + + List candidates = new List(); + FindCandidateInvocationTypes(context, candidates, instanceExpression.ValueType); + + TypeSymbol[] paramTypes = parameterExpressions.Select(ex => ex.ValueType).ToArray(); + + foreach (var candidate in candidates) + { + ExternMethodSymbol externMethodSymbol = new ExternSynthesizedMethodSymbol(context, originalSymbol.Name, candidate, paramTypes, originalSymbol.ReturnType, false, false); + if (CompilerUdonInterface.IsExposedToUdon(externMethodSymbol.ExternSignature)) + { + return externMethodSymbol; + } + } + + return null; + } + + void FindCandidateInvocationTypes(AbstractPhaseContext context, List candidates, TypeSymbol ty) + { + foreach (var intf in ty.RoslynSymbol.AllInterfaces) + { + candidates.Add(context.GetTypeSymbol(intf)); + } + + while (ty != null) + { + candidates.Add(ty); + ty = ty.BaseType; + } } public override Value EmitValue(EmitContext context) @@ -83,7 +133,7 @@ public override Value EmitValue(EmitContext context) module.AddPush(returnValue); } - module.AddExtern((ExternMethodSymbol) Method); + module.AddExtern(externMethodSymbol); if (recursiveValues != null) PopRecursiveValues(recursiveValues, context); diff --git a/Assets/UdonSharp/Editor/Compiler/Binder/BoundNodes/BoundInvocationExpression.cs b/Assets/UdonSharp/Editor/Compiler/Binder/BoundNodes/BoundInvocationExpression.cs index 955f1e56..d1c0f60a 100644 --- a/Assets/UdonSharp/Editor/Compiler/Binder/BoundNodes/BoundInvocationExpression.cs +++ b/Assets/UdonSharp/Editor/Compiler/Binder/BoundNodes/BoundInvocationExpression.cs @@ -164,7 +164,7 @@ private static bool TryCreateInstantiationInvocation(AbstractPhaseContext contex switch (symbol.Name) { case "Instantiate_Extern" when symbol.ContainingType == context.GetTypeSymbol(typeof(InstantiationShim)): - createdInvocation = new BoundExternInvocation(node, + createdInvocation = new BoundExternInvocation(node, context, new ExternSynthesizedMethodSymbol(context, "VRCInstantiate.__Instantiate__UnityEngineGameObject__UnityEngineGameObject", parameterExpressions.Select(e => e.ValueType).ToArray(), @@ -213,7 +213,7 @@ private static bool TryCreateSetProgramVariableInvocation(AbstractPhaseContext c .GetMembers("SetProgramVariable", context) .First(e => !e.RoslynSymbol.IsGenericMethod); - createdInvocation = new BoundExternInvocation(node, setProgramVarObjMethod, instanceExpression, + createdInvocation = new BoundExternInvocation(node, context, setProgramVarObjMethod, instanceExpression, parameterExpressions); return true; } @@ -233,7 +233,7 @@ private static bool TryCreateArrayMethodInvocation(AbstractPhaseContext context, .GetMembers(symbol.Name, context) .First(e => !e.RoslynSymbol.IsGenericMethod && e.Parameters.Length == symbol.Parameters.Length); - createdInvocation = new BoundExternInvocation(node, arrayMethod, instanceExpression, + createdInvocation = new BoundExternInvocation(node, context, arrayMethod, instanceExpression, parameterExpressions); return true; } @@ -249,7 +249,7 @@ private static bool TryCreateTMPMethodInvocation(AbstractPhaseContext context, S if (symbol.ContainingType != null && symbol.ContainingType.ToString() == "TMPro.TMP_Text") { - createdInvocation = new BoundExternInvocation(node, + createdInvocation = new BoundExternInvocation(node, context, new ExternSynthesizedMethodSymbol(context, symbol.Name, instanceExpression.ValueType, symbol.Parameters.Select(e => e.Type).ToArray(), symbol.ReturnType, symbol.IsStatic), instanceExpression, parameterExpressions); @@ -269,7 +269,7 @@ private static bool TryCreateBaseEnumMethodInvocation(AbstractPhaseContext conte symbol.ContainingType != null && symbol.ContainingType == context.GetTypeSymbol(SpecialType.System_Enum)) { - createdInvocation = new BoundExternInvocation(node, + createdInvocation = new BoundExternInvocation(node, context, context.GetTypeSymbol(SpecialType.System_Object).GetMember(symbol.Name, context), instanceExpression, parameterExpressions); @@ -289,7 +289,7 @@ private static bool TryCreateCompareToInvocation(AbstractPhaseContext context, S symbol.ContainingType != null && symbol.ContainingType == context.GetTypeSymbol(typeof(IComparable))) { - createdInvocation = new BoundExternInvocation(node, + createdInvocation = new BoundExternInvocation(node, context, new ExternSynthesizedMethodSymbol(context, "CompareTo", instanceExpression.ValueType, new [] { instanceExpression.ValueType }, context.GetTypeSymbol(SpecialType.System_Int32), false), @@ -344,15 +344,6 @@ public static BoundInvocationExpression CreateBoundInvocation(AbstractPhaseConte if (CompilerUdonInterface.IsUdonEvent(symbol.Name) && symbol.ContainingType == context.GetTypeSymbol(typeof(UdonSharpBehaviour))) // Pass through for making base calls on the U# behaviour type return noop return new BoundUdonSharpBehaviourInvocationExpression(node, symbol, instanceExpression, parameterExpressions); - - bool doExposureCheck = (!symbol.IsOperator || (symbol.ContainingType == null || !symbol.ContainingType.IsEnum)); - - if (symbol.IsOperator && symbol is ExternBuiltinOperatorSymbol operatorSymbol && - operatorSymbol.OperatorType == BuiltinOperatorType.BitwiseNot) - doExposureCheck = false; - - if (doExposureCheck && !CompilerUdonInterface.IsExposedToUdon(((ExternMethodSymbol) symbol).ExternSignature)) - throw new NotExposedException(LocStr.CE_UdonMethodNotExposed, node, $"{symbol.RoslynSymbol?.ToDisplayString() ?? symbol.ToString()}, sig: {((ExternMethodSymbol) symbol).ExternSignature}"); if (symbol.IsOperator) { @@ -362,7 +353,7 @@ public static BoundInvocationExpression CreateBoundInvocation(AbstractPhaseConte MethodSymbol objectEqualsMethod = context.GetTypeSymbol(SpecialType.System_Object) .GetMember("Equals", context); - BoundInvocationExpression boundEqualsInvocation = CreateBoundInvocation(context, node, objectEqualsMethod, parameterExpressions[0], + var boundEqualsInvocation = CreateBoundInvocation(context, node, objectEqualsMethod, parameterExpressions[0], new[] {parameterExpressions[1]}); if (symbol.Name == "op_Equality") return boundEqualsInvocation; @@ -371,7 +362,7 @@ public static BoundInvocationExpression CreateBoundInvocation(AbstractPhaseConte BuiltinOperatorType.UnaryNegation, context.GetTypeSymbol(SpecialType.System_Boolean), context); - return new BoundExternInvocation(node, boolNotOperator, null, new BoundExpression[] {boundEqualsInvocation}); + return new BoundExternInvocation(node, context, boolNotOperator, null, new BoundExpression[] {boundEqualsInvocation}); } if (node is AssignmentExpressionSyntax) @@ -383,13 +374,13 @@ public static BoundInvocationExpression CreateBoundInvocation(AbstractPhaseConte if (parameterExpressions.Length == 2 || symbol.Name == "op_UnaryNegation" || symbol.Name == "op_LogicalNot") { - return new BoundBuiltinOperatorInvocationExpression(node, symbol, parameterExpressions); + return new BoundBuiltinOperatorInvocationExpression(node, context, symbol, parameterExpressions); } throw new NotSupportedException("Operator expressions must have either 1 or 2 parameters", node.GetLocation()); } - return new BoundExternInvocation(node, symbol, instanceExpression, parameterExpressions); + return new BoundExternInvocation(node, context, symbol, instanceExpression, parameterExpressions); } if (symbol.IsStatic) @@ -404,7 +395,7 @@ public static BoundInvocationExpression CreateBoundInvocation(AbstractPhaseConte parameterExpressions); } - throw new NotImplementedException(); + throw new System.NotImplementedException(); } protected override void ReleaseCowValuesImpl(EmitContext context) @@ -596,8 +587,8 @@ protected void PopRecursiveValues(Value[] values, EmitContext context) private sealed class BoundBuiltinOperatorInvocationExpression : BoundExternInvocation { - public BoundBuiltinOperatorInvocationExpression(SyntaxNode node, MethodSymbol method, BoundExpression[] operandExpressions) - :base(node, method, null, operandExpressions) + public BoundBuiltinOperatorInvocationExpression(SyntaxNode node, AbstractPhaseContext context, MethodSymbol method, BoundExpression[] operandExpressions) + :base(node, context, method, null, operandExpressions) { } } @@ -642,7 +633,7 @@ private sealed class BoundGetUnityEngineComponentInvocation : BoundExternInvocat public override TypeSymbol ValueType { get; } public BoundGetUnityEngineComponentInvocation(AbstractPhaseContext context, SyntaxNode node, MethodSymbol methodSymbol, BoundExpression sourceExpression, BoundExpression[] parametersExpressions) - : base(node, BuildMethod(context, methodSymbol), sourceExpression, GetParameterExpressions(context, methodSymbol, parametersExpressions)) + : base(node, context, BuildMethod(context, methodSymbol), sourceExpression, GetParameterExpressions(context, methodSymbol, parametersExpressions)) { ValueType = methodSymbol.TypeArguments[0]; diff --git a/Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.asset b/Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.asset new file mode 100644 index 00000000..e244e27a --- /dev/null +++ b/Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.asset @@ -0,0 +1,120 @@ +%YAML 1.1 +%TAG !u! tag:unity3d.com,2011: +--- !u!114 &11400000 +MonoBehaviour: + m_ObjectHideFlags: 0 + m_CorrespondingSourceObject: {fileID: 0} + m_PrefabInstance: {fileID: 0} + m_PrefabAsset: {fileID: 0} + m_GameObject: {fileID: 0} + m_Enabled: 1 + m_EditorHideFlags: 0 + m_Script: {fileID: 11500000, guid: c333ccfdd0cbdbc4ca30cef2dd6e6b9b, type: 3} + m_Name: AccessViaAlternateInvocee + m_EditorClassIdentifier: + serializedUdonProgramAsset: {fileID: 11400000, guid: 8876b3583cec99942b84819c559e175c, + type: 2} + udonAssembly: + assemblyError: + sourceCsScript: {fileID: 11500000, guid: f4acb09f599aab7418e27fcf7aff58e3, type: 3} + behaviourSyncMode: 0 + compileErrors: [] + hasInteractEvent: 0 + serializationData: + SerializedFormat: 2 + SerializedBytes: + ReferencedUnityObjects: [] + SerializedBytesString: + Prefab: {fileID: 0} + PrefabModificationsReferencedUnityObjects: [] + PrefabModifications: [] + SerializationNodes: + - Name: fieldDefinitions + Entry: 7 + Data: 0|System.Collections.Generic.Dictionary`2[[System.String, mscorlib],[UdonSharp.Compiler.FieldDefinition, + UdonSharp.Editor]], mscorlib + - Name: comparer + Entry: 7 + Data: 1|System.Collections.Generic.GenericEqualityComparer`1[[System.String, + mscorlib]], mscorlib + - Name: + Entry: 8 + Data: + - Name: + Entry: 12 + Data: 1 + - Name: + Entry: 7 + Data: + - Name: $k + Entry: 1 + Data: tester + - Name: $v + Entry: 7 + Data: 2|UdonSharp.Compiler.FieldDefinition, UdonSharp.Editor + - Name: k__BackingField + Entry: 1 + Data: tester + - Name: k__BackingField + Entry: 7 + Data: 3|System.RuntimeType, mscorlib + - Name: + Entry: 1 + Data: UdonSharp.Tests.IntegrationTestSuite, Assembly-CSharp + - Name: + Entry: 8 + Data: + - Name: k__BackingField + Entry: 7 + Data: 4|System.RuntimeType, mscorlib + - Name: + Entry: 1 + Data: VRC.Udon.UdonBehaviour, VRC.Udon + - Name: + Entry: 8 + Data: + - Name: k__BackingField + Entry: 7 + Data: System.Nullable`1[[UdonSharp.UdonSyncMode, UdonSharp.Runtime]], mscorlib + - Name: + Entry: 6 + Data: + - Name: + Entry: 8 + Data: + - Name: k__BackingField + Entry: 5 + Data: false + - Name: fieldAttributes + Entry: 7 + Data: 5|System.Collections.Generic.List`1[[System.Attribute, mscorlib]], mscorlib + - Name: + Entry: 12 + Data: 1 + - Name: + Entry: 7 + Data: 6|System.NonSerializedAttribute, mscorlib + - Name: + Entry: 8 + Data: + - Name: + Entry: 13 + Data: + - Name: + Entry: 8 + Data: + - Name: userBehaviourSource + Entry: 6 + Data: + - Name: + Entry: 8 + Data: + - Name: + Entry: 8 + Data: + - Name: + Entry: 13 + Data: + - Name: + Entry: 8 + Data: diff --git a/Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.asset.meta b/Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.asset.meta new file mode 100644 index 00000000..87ef6d4c --- /dev/null +++ b/Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.asset.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: a2e5c6eb44b8f874cbb7622dd67fff5e +NativeFormatImporter: + externalObjects: {} + mainObjectFileID: 11400000 + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.cs b/Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.cs new file mode 100644 index 00000000..378a7663 --- /dev/null +++ b/Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.cs @@ -0,0 +1,26 @@ + +using UdonSharp; +using UnityEngine; +using VRC.SDKBase; +using VRC.Udon; + +namespace UdonSharp.Tests +{ + /// + /// Certain Udon externs are exposed not via the C# type or interface they're implemented on, but rather + /// on some subclass directly. This test verifies that we can search for such an alternate subclass and + /// use it for the extern invocation. + /// + [AddComponentMenu("Udon Sharp/Tests/AccessViaAlternateInvocee")] + public class AccessViaAlternateInvocee : UdonSharpBehaviour + { + [System.NonSerialized] + public IntegrationTestSuite tester; + + public void ExecuteTests() + { + System.Type tyString = typeof(string); + tester.TestAssertion("Access System.Type.Name", tyString.Name.Equals("String")); + } + } +} diff --git a/Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.cs.meta b/Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.cs.meta new file mode 100644 index 00000000..9625eb4e --- /dev/null +++ b/Assets/UdonSharp/Tests~/TestScripts/BugTests/AccessViaAlternateInvocee/AccessViaAlternateInvocee.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: f4acb09f599aab7418e27fcf7aff58e3 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: