Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cancel unnecessary constructors #1185

Merged
merged 4 commits into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ private void ProcessMethod(SemanticModel model, IMethodSymbol symbol, bool expor
}

MethodConvert convert = ConvertMethod(model, symbol);
if (export && !symbol.IsStatic)
if (export && MethodConvert.NeedInstanceConstructor(symbol))
{
MethodConvert forward = new(this, symbol);
forward.ConvertForward(model, convert);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class CompilationEngine(CompilationOptions options)
internal CompilationOptions Options { get; private set; } = options;
private static readonly MetadataReference[] CommonReferences;
private static readonly Dictionary<string, MetadataReference> MetaReferences = [];
private static readonly Regex s_pattern = new(@"^(Neo\.SmartContract\.Framework\.SmartContract|SmartContract\.Framework\.SmartContract|Framework\.SmartContract|SmartContract)$");
internal static readonly Regex s_pattern = new(@"^(Neo\.SmartContract\.Framework\.SmartContract|SmartContract\.Framework\.SmartContract|Framework\.SmartContract|SmartContract|Neo\.SmartContract\.Framework\.Nep17Token|Neo\.SmartContract\.Framework\.TokenContract|Neo.SmartContract.Framework.Nep11Token<.*>)$");
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should try to remove this regex

Copy link
Member

Choose a reason for hiding this comment

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

@Hecate2 this is needed? it should check the hierarchy

Copy link
Member

Choose a reason for hiding this comment

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

  static bool IsDerivedFromSmartContract(INamedTypeSymbol classSymbol, Regex pattern)
  {
      var baseType = classSymbol.BaseType;
      while (baseType != null)
      {
          if (pattern.IsMatch(baseType.ToString() ?? string.Empty))
          {
              return true;
          }
          baseType = baseType.BaseType;
      }
      return false;
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IsDerivedFromSmartContract returns true even when a asmart contract is derived for many times. e.g.
class A: SmartContract
class B: A
class C: B
IsDerivedFromSmartContract returns true for B and C. But in B and C we may call base, or utilize the constructor of A.

internal readonly ConcurrentDictionary<INamedTypeSymbol, CompilationContext> Contexts = new(SymbolEqualityComparer.Default);

static CompilationEngine()
Expand Down Expand Up @@ -319,7 +319,7 @@ void Visit(INamedTypeSymbol classSymbol)
return sorted;
}

static bool IsDerivedFromSmartContract(INamedTypeSymbol classSymbol, Regex pattern)
internal static bool IsDerivedFromSmartContract(INamedTypeSymbol classSymbol, Regex pattern)
{
var baseType = classSymbol.BaseType;
while (baseType != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ private void ConvertIdentifierNameAssignment(SemanticModel model, IdentifierName
}
else if (property.SetMethod != null)
{
if (!property.IsStatic) AddInstruction(OpCode.LDARG0);
if (NeedInstanceConstructor(property.SetMethod)) AddInstruction(OpCode.LDARG0);
CallMethodWithConvention(model, property.SetMethod, CallingConvention.Cdecl);
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ private void ConvertIdentifierNameExpression(SemanticModel model, IdentifierName
if (!_internalInline) LdArgSlot(parameter);
break;
case IPropertySymbol property:
if (!property.IsStatic)
if (NeedInstanceConstructor(property.GetMethod!))
AddInstruction(OpCode.LDARG0);
CallMethodWithConvention(model, property.GetMethod!);
break;
Expand Down
4 changes: 2 additions & 2 deletions src/Neo.Compiler.CSharp/MethodConvert/Helpers/CallHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ private void CallMethodWithConvention(SemanticModel model, IMethodSymbol symbol,

var (convert, methodCallingConvention) = GetMethodConvertAndCallingConvention(model, symbol);

int pc = symbol.Parameters.Length + (symbol.IsStatic ? 0 : 1);
int pc = symbol.Parameters.Length + (!NeedInstanceConstructor(symbol) ? 0 : 1);
if (pc > 1 && methodCallingConvention != callingConvention)
ReverseStackItems(pc);

Expand Down Expand Up @@ -255,7 +255,7 @@ private void HandleConstructorDuplication(bool instanceOnStack, CallingConventio
private void HandleInstanceExpression(SemanticModel model, IMethodSymbol symbol, ExpressionSyntax? instanceExpression,
CallingConvention methodCallingConvention, bool beforeArguments)
{
if (symbol.IsStatic) return;
if (!NeedInstanceConstructor(symbol)) return;

bool shouldConvert = beforeArguments ? (methodCallingConvention != CallingConvention.Cdecl)
: (methodCallingConvention == CallingConvention.Cdecl);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ private void InsertInitializationInstructions()
if (!_initSlot) return;
byte pc = (byte)_parameters.Count;
byte lc = (byte)_localsCount;
if (IsInstanceMethod(Symbol)) pc++;
if (NeedInstanceConstructor(Symbol)) pc++;
// Only add INITSLOT if we have local variables or parameters
if (pc > 0 || lc > 0)
{
Expand Down
11 changes: 7 additions & 4 deletions src/Neo.Compiler.CSharp/MethodConvert/PropertyConvert.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ private void ConvertFieldBackedProperty(IPropertySymbol property)
}
else
{
if (!NeedInstanceConstructor(Symbol))
return;
fields = fields.Where(p => !p.IsStatic).ToArray();
int backingFieldIndex = Array.FindIndex(fields, p => SymbolEqualityComparer.Default.Equals(p.AssociatedSymbol, property));
switch (Symbol.MethodKind)
Expand Down Expand Up @@ -130,7 +132,7 @@ private void ConvertStorageBackedProperty(IPropertySymbol property, AttributeDat
// Ensure that no object was sent
Jump(OpCode.JMPIFNOT_L, endTarget);
}
else
else if (NeedInstanceConstructor(Symbol))
{
// Check class
Jump(OpCode.JMPIF_L, endTarget);
Expand Down Expand Up @@ -181,15 +183,16 @@ private void ConvertStorageBackedProperty(IPropertySymbol property, AttributeDat
CallContractMethod(NativeContract.StdLib.Hash, "deserialize", 1, true);
break;
}
AddInstruction(OpCode.DUP);
if (Symbol.IsStatic)
{
AddInstruction(OpCode.DUP);
IFieldSymbol backingField = Array.Find(fields, p => SymbolEqualityComparer.Default.Equals(p.AssociatedSymbol, property))!;
byte backingFieldIndex = _context.AddStaticField(backingField);
AccessSlot(OpCode.STSFLD, backingFieldIndex);
}
else
else if (NeedInstanceConstructor(Symbol))
{
AddInstruction(OpCode.DUP);
fields = fields.Where(p => !p.IsStatic).ToArray();
int backingFieldIndex = Array.FindIndex(fields, p => SymbolEqualityComparer.Default.Equals(p.AssociatedSymbol, property));
AccessSlot(OpCode.LDARG, 0);
Expand All @@ -201,7 +204,7 @@ private void ConvertStorageBackedProperty(IPropertySymbol property, AttributeDat
}
else
{
if (Symbol.IsStatic)
if (Symbol.IsStatic || !NeedInstanceConstructor(Symbol))
AccessSlot(OpCode.LDARG, 0);
else
AccessSlot(OpCode.LDARG, 1);
Expand Down
40 changes: 37 additions & 3 deletions src/Neo.Compiler.CSharp/MethodConvert/SourceConvert.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Neo.VM;
using System.Linq;

namespace Neo.Compiler;

Expand All @@ -24,7 +25,7 @@ private void ConvertSource(SemanticModel model)
{
IParameterSymbol parameter = Symbol.Parameters[i].OriginalDefinition;
byte index = i;
if (IsInstanceMethod(Symbol)) index++;
if (NeedInstanceConstructor(Symbol)) index++;
_parameters.Add(parameter, index);

if (parameter.RefKind == RefKind.Out)
Expand Down Expand Up @@ -139,6 +140,39 @@ private static bool IsExpressionReturningValue(SemanticModel semanticModel, Meth
return false;
}

private bool IsInstanceMethod(IMethodSymbol symbol) => !symbol.IsStatic && symbol.MethodKind != MethodKind.AnonymousFunction;

/// <summary>
/// non-static methods needs constructors to be executed
/// But non-static method in smart contract classes without explicit constructor
/// does not constructors
/// Cases we need instance constructors:
/// 1. non-static smart contract with explicit instance constructor in itself
/// 2. non-static ordinary method, with explicit instance constructor in itself or its base classes
/// </summary>
/// <param name="symbol">A method in class</param>
/// <returns></returns>
internal static bool NeedInstanceConstructor(IMethodSymbol symbol)
{
if (symbol.IsStatic || symbol.MethodKind == MethodKind.AnonymousFunction)
return false;
INamedTypeSymbol? containingClass = symbol.ContainingType;
if (containingClass == null) return false;
// non-static methods in class
if ((symbol.MethodKind == MethodKind.Constructor || symbol.MethodKind == MethodKind.SharedConstructor)
&& !CompilationEngine.IsDerivedFromSmartContract(containingClass, CompilationEngine.s_pattern))
// is constructor, and is not smart contract
// typically seen in framework methods
return true;
// is smart contract, or is normal non-static method (whether contract or not)
if (containingClass?.Constructors
.FirstOrDefault(p => p.Parameters.Length == 0 && !p.IsStatic)?
.DeclaringSyntaxReferences.Length == 0)
// No explicit non-static constructor in class
{
if (CompilationEngine.s_pattern.IsMatch(containingClass.BaseType?.ToString() ?? string.Empty))
// class itself is directly inheriting smart contract; cannot have more base classes
return false;
}
// has explicit constructor
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ public abstract class Contract_Abort(Neo.SmartContract.Testing.SmartContractInit
{
#region Compiled data

public static Neo.SmartContract.Manifest.ContractManifest Manifest => Neo.SmartContract.Manifest.ContractManifest.Parse(@"{""name"":""Contract_Abort"",""groups"":[],""features"":{},""supportedstandards"":[],""abi"":{""methods"":[{""name"":""testAbort"",""parameters"":[],""returntype"":""Integer"",""offset"":148,""safe"":false},{""name"":""testAbortMsg"",""parameters"":[],""returntype"":""Integer"",""offset"":160,""safe"":false},{""name"":""testAbortInFunction"",""parameters"":[{""name"":""abortMsg"",""type"":""Boolean""}],""returntype"":""Integer"",""offset"":172,""safe"":false},{""name"":""testAbortInTry"",""parameters"":[{""name"":""abortMsg"",""type"":""Boolean""}],""returntype"":""Integer"",""offset"":184,""safe"":false},{""name"":""testAbortInCatch"",""parameters"":[{""name"":""abortMsg"",""type"":""Boolean""}],""returntype"":""Integer"",""offset"":196,""safe"":false},{""name"":""testAbortInFinally"",""parameters"":[{""name"":""abortMsg"",""type"":""Boolean""}],""returntype"":""Integer"",""offset"":208,""safe"":false}],""events"":[]},""permissions"":[],""trusts"":[],""extra"":{""nef"":{""optimization"":""All""}}}");
public static Neo.SmartContract.Manifest.ContractManifest Manifest => Neo.SmartContract.Manifest.ContractManifest.Parse(@"{""name"":""Contract_Abort"",""groups"":[],""features"":{},""supportedstandards"":[],""abi"":{""methods"":[{""name"":""testAbort"",""parameters"":[],""returntype"":""Integer"",""offset"":0,""safe"":false},{""name"":""testAbortMsg"",""parameters"":[],""returntype"":""Integer"",""offset"":6,""safe"":false},{""name"":""testAbortInFunction"",""parameters"":[{""name"":""abortMsg"",""type"":""Boolean""}],""returntype"":""Integer"",""offset"":23,""safe"":false},{""name"":""testAbortInTry"",""parameters"":[{""name"":""abortMsg"",""type"":""Boolean""}],""returntype"":""Integer"",""offset"":35,""safe"":false},{""name"":""testAbortInCatch"",""parameters"":[{""name"":""abortMsg"",""type"":""Boolean""}],""returntype"":""Integer"",""offset"":60,""safe"":false},{""name"":""testAbortInFinally"",""parameters"":[{""name"":""abortMsg"",""type"":""Boolean""}],""returntype"":""Integer"",""offset"":96,""safe"":false}],""events"":[]},""permissions"":[],""trusts"":[],""extra"":{""nef"":{""optimization"":""All""}}}");

/// <summary>
/// Optimization: "All"
/// </summary>
public static Neo.SmartContract.NefFile Nef => Neo.IO.Helper.AsSerializable<Neo.SmartContract.NefFile>(Convert.FromBase64String(@"TkVGM1Rlc3RpbmdFbmdpbmUAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAANpXAQEQcDhXAAF4NANAVwABQFcBARBwDAlBQk9SVCBNU0fgVwECEHB5JgV4NOZ4NNJXAgIQcDsMEXkmBXg01Xg0wXERcD0FEnA/aEBXAgIQcDsRIRFwDAlleGNlcHRpb246cXkmCHg1q////3g1lP///xJwP1cCAhBwOwcMEXA9AHEScD0AeSYIeDWI////eDVx////wko1cP///yNl////wko1ZP///yNq////wko1WP///yNv////wko1TP///yNx////wko1QP///yOA////wko1NP///yKdQCgI690="));
public static Neo.SmartContract.NefFile Nef => Neo.IO.Helper.AsSerializable<Neo.SmartContract.NefFile>(Convert.FromBase64String(@"TkVGM1Rlc3RpbmdFbmdpbmUAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAH9XAQAQcDhXAQAQcAwJQUJPUlQgTVNH4FcBARBweCYENOc031cCARBwOwoPeCYENNg00HERcD0FEnA/aEBXAgEQcDsRHBFwDAlleGNlcHRpb246cXgmBDSwNaj///8ScD9XAgEQcDsHDBFwPQBxEnA9AHgmBzWS////NYf///9Auddq9Q=="));

#endregion

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ public abstract class Contract_Assert(Neo.SmartContract.Testing.SmartContractIni
{
#region Compiled data

public static Neo.SmartContract.Manifest.ContractManifest Manifest => Neo.SmartContract.Manifest.ContractManifest.Parse(@"{""name"":""Contract_Assert"",""groups"":[],""features"":{},""supportedstandards"":[],""abi"":{""methods"":[{""name"":""testAssertFalse"",""parameters"":[],""returntype"":""Integer"",""offset"":121,""safe"":false},{""name"":""testAssertInFunction"",""parameters"":[],""returntype"":""Integer"",""offset"":133,""safe"":false},{""name"":""testAssertInTry"",""parameters"":[],""returntype"":""Integer"",""offset"":145,""safe"":false},{""name"":""testAssertInCatch"",""parameters"":[],""returntype"":""Integer"",""offset"":157,""safe"":false},{""name"":""testAssertInFinally"",""parameters"":[],""returntype"":""Integer"",""offset"":166,""safe"":false}],""events"":[]},""permissions"":[],""trusts"":[],""extra"":{""nef"":{""optimization"":""All""}}}");
public static Neo.SmartContract.Manifest.ContractManifest Manifest => Neo.SmartContract.Manifest.ContractManifest.Parse(@"{""name"":""Contract_Assert"",""groups"":[],""features"":{},""supportedstandards"":[],""abi"":{""methods"":[{""name"":""testAssertFalse"",""parameters"":[],""returntype"":""Integer"",""offset"":0,""safe"":false},{""name"":""testAssertInFunction"",""parameters"":[],""returntype"":""Integer"",""offset"":16,""safe"":false},{""name"":""testAssertInTry"",""parameters"":[],""returntype"":""Integer"",""offset"":28,""safe"":false},{""name"":""testAssertInCatch"",""parameters"":[],""returntype"":""Integer"",""offset"":51,""safe"":false},{""name"":""testAssertInFinally"",""parameters"":[],""returntype"":""Integer"",""offset"":84,""safe"":false}],""events"":[]},""permissions"":[],""trusts"":[],""extra"":{""nef"":{""optimization"":""All""}}}");

/// <summary>
/// Optimization: "All"
/// </summary>
public static Neo.SmartContract.NefFile Nef => Neo.IO.Helper.AsSerializable<Neo.SmartContract.NefFile>(Convert.FromBase64String(@"TkVGM1Rlc3RpbmdFbmdpbmUAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAALBXAQEQcAg5EXAJOQBkcGhAVwABeDQDQFcAAUBXAQEQcHg033ARcGhAVwIBEHA7CQ54NM9wPQpxEXA9BRJwP2hAVwIBEHA7ERgRcAwJZXhjZXB0aW9uOnF4NKhwPQUScD9oQFcCARBwOwcMEXA9AHEScD0AeDWM////wko1lf///yOA////wko1if///yOP////wko1ff///yOQ////wko1cf///yKcwko1aP///yK1QF5bPYA="));
public static Neo.SmartContract.NefFile Nef => Neo.IO.Helper.AsSerializable<Neo.SmartContract.NefFile>(Convert.FromBase64String(@"TkVGM1Rlc3RpbmdFbmdpbmUAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAGtXAQAQcAg5EXAJOQBkcGhAVwEAEHA063ARcGhAVwIAEHA7CA003HA9CnERcD0FEnA/aEBXAgAQcDsRFxFwDAlleGNlcHRpb246cTS2cD0FEnA/aEBXAgAQcDsHDBFwPQBxEnA9ADWb////QJ1a8Js="));

#endregion

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ public abstract class Contract_Delegate(Neo.SmartContract.Testing.SmartContractI
{
#region Compiled data

public static Neo.SmartContract.Manifest.ContractManifest Manifest => Neo.SmartContract.Manifest.ContractManifest.Parse(@"{""name"":""Contract_Delegate"",""groups"":[],""features"":{},""supportedstandards"":[],""abi"":{""methods"":[{""name"":""sumFunc"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":0,""safe"":false},{""name"":""testDelegate"",""parameters"":[],""returntype"":""Void"",""offset"":163,""safe"":false}],""events"":[]},""permissions"":[{""contract"":""0xacce6fd80d44e1796aa0c2c625e9e4e0ce39efc0"",""methods"":[""itoa""]}],""trusts"":[],""extra"":{""nef"":{""optimization"":""All""}}}");
public static Neo.SmartContract.Manifest.ContractManifest Manifest => Neo.SmartContract.Manifest.ContractManifest.Parse(@"{""name"":""Contract_Delegate"",""groups"":[],""features"":{},""supportedstandards"":[],""abi"":{""methods"":[{""name"":""sumFunc"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":0,""safe"":false},{""name"":""testDelegate"",""parameters"":[],""returntype"":""Void"",""offset"":118,""safe"":false}],""events"":[]},""permissions"":[{""contract"":""0xacce6fd80d44e1796aa0c2c625e9e4e0ce39efc0"",""methods"":[""itoa""]}],""trusts"":[],""extra"":{""nef"":{""optimization"":""All""}}}");

/// <summary>
/// Optimization: "All"
/// </summary>
public static Neo.SmartContract.NefFile Nef => Neo.IO.Helper.AsSerializable<Neo.SmartContract.NefFile>(Convert.FromBase64String(@"TkVGM1Rlc3RpbmdFbmdpbmUAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHA7znO4OTpJcbCoGp54UQN2G/OrARpdG9hAQABDwAAqlcAAnl4CgcAAAA2QFcAAnh5nkoCAAAAgC4EIgpKAv///38yHgP/////AAAAAJFKAv///38yDAMAAAAAAQAAAJ9AVwACeHmeSgIAAACALgQiCkoC////fzIeA/////8AAAAAkUoC////fzIMAwAAAAABAAAAn0BXAgEKyP///3AWFWg2cQwFU3VtOiBpNwAAi9soQc/nR5ZAVwABeDQDQFcAAUDCSjTzIs9AJ9ukPQ=="));
public static Neo.SmartContract.NefFile Nef => Neo.IO.Helper.AsSerializable<Neo.SmartContract.NefFile>(Convert.FromBase64String(@"TkVGM1Rlc3RpbmdFbmdpbmUAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHA7znO4OTpJcbCoGp54UQN2G/OrARpdG9hAQABDwAAmFcAAnl4CgcAAAA2QFcAAnh5nkoCAAAAgC4EIgpKAv///38yHgP/////AAAAAJFKAv///38yDAMAAAAAAQAAAJ9AVwACeHmeSgIAAACALgQiCkoC////fzIeA/////8AAAAAkUoC////fzIMAwAAAAABAAAAn0BXAgAKyP///3AWFWg2cQwFU3VtOiBpNwAAi9soQc/nR5ZA1O3dOg=="));

#endregion

Expand Down
Loading
Loading