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

Implementation of LDFN & CALL_SP, and some fixes delegate calling #251

Merged
merged 13 commits into from
Jun 30, 2024

Conversation

0xF6
Copy link
Member

@0xF6 0xF6 commented Jun 30, 2024

Summary by CodeRabbit

  • New Features

    • Added TYPE_NULL to the VeinTypeCode enum.
    • Introduced new opcodes LDFN and CALL_SP for method handling.
    • Added new memory allocation methods AllocRawValue and FreeRawValue in the Ishtar Garbage Collector.
    • Introduced structures rawval_union, VeinRawCode, and rawval for advanced memory management.
  • Improvements

    • Enhanced compatibility checks with additional parameters and improved argument handling.
    • Improved argument validation and exception handling logic in virtual machine methods.
    • Added new boolean property DisableValidationInvocationArgs for flexible validation control.
  • Bug Fixes

    • Addressed null handling in UnBoxing method to prevent runtime errors.
  • Refactor

    • Updated method signatures and logic for better code clarity and maintenance.

Copy link
Contributor

coderabbitai bot commented Jun 30, 2024

Walkthrough

Overall, the changes enhance method compatibility checks, introduce new enum values, improve code generation and runtime handling, and refine memory management. The updates focus on adding arguments, refining logic for argument handling, enhancing exception handling, and introducing new memory allocation methods.

Changes

File Change Summary
.../common/reflection/VeinClass.cs Modified CheckCompatibilityFunctionClass to pass an additional boolean argument to HasCompatibility
.../common/reflection/VeinMethod.cs Updated HasCompatibility to include a new ignoreThis parameter and adjusted compatibility logic accordingly
.../common/reflection/VeinTypeCode.cs Added a new enum value TYPE_NULL to VeinTypeCode
.../ishtar.generator/GeneratorContext.cs Adjusted method signatures and parameter handling in CreateFunctionMulticastGroup, including changes in code generation logic
.../ishtar.generator/generators/call.cs Modified EmitCall and IsCallingDelegate methods to include additional arguments and an instruction emission
.../ishtar.generator/generators/operators.cs Removed gen.Emit(OpCodes.LDNULL); and modified parameters in call to FindMethod
.../ishtar.vm/VirtualMachine.cs Added argument validation logic before method execution, replaced ForceFail, and introduced new opcodes and enhanced exception handling
.../ishtar.vm/runtime/IshtarMarshal.cs Updated UnBoxing to handle null input by returning a stackval with type = TYPE_NULL and added a default case throwing NotImplementedException
.../ishtar.vm/runtime/gc/IshtarGC.cs Added new methods AllocRawValue and FreeRawValue for memory allocation and deallocation
.../ishtar.vm/runtime/vm/CallFrame.cs Added a check in FillStackTrace method before assigning to frame->exception.stack_trace
.../ishtar.vm/stackval.cs Added new structures and methods for memory allocation including rawval_union, VeinRawCode, and rawval
.../ishtar.vm/vm.switch.cs Added a new boolean property DisableValidationInvocationArgs for validation control

Poem

In code realms where logic blooms,
We shaped new paths with digital plumes.
Methods check with finer grace,
Nulls are caught in memory's embrace.
Validation shines in fresh delight,
As bytes dance in the VM's light.
Here’s to progress, pure and bright! 🌟


Tip

Early access features: enabled

We are currently testing the following features in early access:

  • OpenAI gpt-4o model for code reviews and chat: OpenAI claims that this model is better at understanding and generating code than the previous models. We seek your feedback over the next few weeks before making it generally available.

Note:

  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.
  • Please join our Discord Community to provide feedback and report issues.
  • OSS projects are always opted into early access features.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between cbf7fe5 and e2ddc48.

Files selected for processing (12)
  • runtime/common/reflection/VeinClass.cs (1 hunks)
  • runtime/common/reflection/VeinMethod.cs (1 hunks)
  • runtime/common/reflection/VeinTypeCode.cs (1 hunks)
  • runtime/ishtar.generator/GeneratorContext.cs (2 hunks)
  • runtime/ishtar.generator/generators/call.cs (2 hunks)
  • runtime/ishtar.generator/generators/operators.cs (2 hunks)
  • runtime/ishtar.vm/VirtualMachine.cs (12 hunks)
  • runtime/ishtar.vm/runtime/IshtarMarshal.cs (2 hunks)
  • runtime/ishtar.vm/runtime/gc/IshtarGC.cs (3 hunks)
  • runtime/ishtar.vm/runtime/vm/CallFrame.cs (2 hunks)
  • runtime/ishtar.vm/stackval.cs (1 hunks)
  • runtime/ishtar.vm/vm.switch.cs (1 hunks)
Files skipped from review due to trivial changes (1)
  • runtime/ishtar.vm/vm.switch.cs
Additional context used
GitHub Check: Codacy Static Code Analysis
runtime/ishtar.vm/stackval.cs

[warning] 9-9: runtime/ishtar.vm/stackval.cs#L9
Implement 'IEquatable' in value type 'rawval_union'.


[warning] 21-21: runtime/ishtar.vm/stackval.cs#L21
Implement 'IEquatable' in value type 'rawval'.


[warning] 34-34: runtime/ishtar.vm/stackval.cs#L34
Remove this unused method parameter 'frame'.


[warning] 36-36: runtime/ishtar.vm/stackval.cs#L36
Remove this unused method parameter 'frame'.


[warning] 39-39: runtime/ishtar.vm/stackval.cs#L39
Remove this unused method parameter 'size'.


[warning] 41-41: runtime/ishtar.vm/stackval.cs#L41
Remove this unused method parameter 'size'.

runtime/ishtar.generator/GeneratorContext.cs

[warning] 202-202: runtime/ishtar.generator/GeneratorContext.cs#L202
Remove this commented out code.


[warning] 205-205: runtime/ishtar.generator/GeneratorContext.cs#L205
Remove this commented out code.

runtime/ishtar.vm/VirtualMachine.cs

[notice] 743-743: runtime/ishtar.vm/VirtualMachine.cs#L743
Remove the unused local variable 'sw'.

GitHub Check: build_all (ubuntu-latest, linux-x64, true)
runtime/ishtar.vm/stackval.cs

[warning] 55-55:
The type name 'stackval' only contains lower-cased ascii characters. Such names may become reserved for the language.


[warning] 21-21:
The type name 'rawval' only contains lower-cased ascii characters. Such names may become reserved for the language.

GitHub Check: build_all (macos-latest, osx-x64, false)
runtime/ishtar.vm/stackval.cs

[warning] 21-21:
The type name 'rawval' only contains lower-cased ascii characters. Such names may become reserved for the language.


[warning] 55-55:
The type name 'stackval' only contains lower-cased ascii characters. Such names may become reserved for the language.

Additional comments not posted (21)
runtime/ishtar.generator/generators/call.cs (2)

46-51: Ensure correct setting of index parameter.

The index parameter is set based on the arguments. Ensure that it is correctly set and used.

Verification successful

The index parameter is correctly set and used.

The index parameter is set based on the arguments of the current method's signature and is correctly filtered and matched with the invocation's expression string.

  • runtime/ishtar.generator/generators/call.cs: Lines 46-51
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the correctness of the `index` parameter setting.

# Test: Search for the `IsCallingDelegate` method definition. Expect: Correct setting of `index` parameter.
rg --type python $'IsCallingDelegate'

Length of output: 67



Script:

#!/bin/bash
# Description: Verify the correctness of the `index` parameter setting in C# files.

# Test: Search for the `IsCallingDelegate` method definition. Expect: Correct setting of `index` parameter.
rg --type cs 'IsCallingDelegate'

Length of output: 343



Script:

#!/bin/bash
# Description: Extract and display lines around the `IsCallingDelegate` method definition in `runtime/ishtar.generator/generators/call.cs`.

# Extract lines around the method definition.
rg --type cs -A 10 -B 10 'public static bool IsCallingDelegate' runtime/ishtar.generator/generators/call.cs

Length of output: 831



Script:

#!/bin/bash
# Description: Extract more lines to capture the complete implementation of `IsCallingDelegate` in `runtime/ishtar.generator/generators/call.cs`.

# Extract additional lines around the method definition.
rg --type cs -A 20 -B 20 'public static bool IsCallingDelegate' runtime/ishtar.generator/generators/call.cs

Length of output: 1440


16-19: Ensure correct usage of index parameter.

The index parameter is used to load the argument. Ensure that it is correctly calculated and used.

Verification successful

Ensure correct usage of index parameter.

The index parameter is correctly calculated in the IsCallingDelegate method and used appropriately in the EmitLoadArgument function.

  • runtime/ishtar.generator/generators/call.cs
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the correctness of the `index` parameter usage.

# Test: Search for the `IsCallingDelegate` method usage. Expect: Correct usage of `index` parameter.
rg --type python $'IsCallingDelegate'

Length of output: 67



Script:

#!/bin/bash
# Description: Verify the correctness of the `index` parameter usage.

# Test: Search for the `IsCallingDelegate` method usage in C# files. Expect: Correct usage of `index` parameter.
rg --type cs 'IsCallingDelegate'

Length of output: 343



Script:

#!/bin/bash
# Extract the definition and usage of the IsCallingDelegate method in call.cs

# Print the definition of IsCallingDelegate
rg --type cs 'public static bool IsCallingDelegate' -A 10 runtime/ishtar.generator/generators/call.cs

# Print the usage of IsCallingDelegate
rg --type cs 'ctx.IsCallingDelegate' -A 5 runtime/ishtar.generator/generators/call.cs

Length of output: 864

runtime/ishtar.vm/runtime/vm/CallFrame.cs (1)

117-118: Ensure correct null check for stack_trace.

The stack_trace is checked for null before assigning a new value. Ensure that this check is correctly implemented.

Verification successful

Verification of stack_trace null check is successful.

The FillStackTrace method in CallFrame.cs includes a check to ensure that stack_trace is null before assigning a new value:

if (frame->exception.stack_trace is null)
    frame->exception.stack_trace = StringStorage.Intern(str.ToString());

This confirms that the null check is correctly implemented.

  • File: runtime/ishtar.vm/runtime/vm/CallFrame.cs
  • Method: FillStackTrace
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the correctness of the null check for `stack_trace`.

# Test: Search for the `FillStackTrace` method definition. Expect: Correct null check for `stack_trace`.
rg --type python $'FillStackTrace'

Length of output: 64



Script:

#!/bin/bash
# Description: Verify the correctness of the null check for `stack_trace`.

# Test: Search for the `FillStackTrace` method definition in C# files. Expect: Correct null check for `stack_trace`.
rg --type cs 'FillStackTrace'

Length of output: 727



Script:

#!/bin/bash
# Description: Extract the implementation of the FillStackTrace method in CallFrame.cs to verify the null check for stack_trace.

# Extract and display the FillStackTrace method implementation.
rg --type cs --context 10 'public static void FillStackTrace(CallFrame* frame)' runtime/ishtar.vm/runtime/vm/CallFrame.cs

Length of output: 123



Script:

#!/bin/bash
# Description: Extract the implementation of the FillStackTrace method in CallFrame.cs to verify the null check for stack_trace.

# Extract and display the FillStackTrace method implementation with a broader context.
rg --type cs --context 20 'public static void FillStackTrace(CallFrame* frame)' runtime/ishtar.vm/runtime/vm/CallFrame.cs

Length of output: 123



Script:

#!/bin/bash
# Description: Extract the implementation of the FillStackTrace method in CallFrame.cs to verify the null check for stack_trace.

# Extract and display the FillStackTrace method implementation without specifying context lines.
rg --type cs 'public static void FillStackTrace(CallFrame* frame)' runtime/ishtar.vm/runtime/vm/CallFrame.cs

Length of output: 110



Script:

#!/bin/bash
# Description: Search for all method definitions in CallFrame.cs to verify the presence and implementation of the FillStackTrace method.

# Search for all method definitions in CallFrame.cs.
rg --type cs 'public static void' runtime/ishtar.vm/runtime/vm/CallFrame.cs

Length of output: 137



Script:

#!/bin/bash
# Description: Extract the entire content of CallFrame.cs to verify the implementation of the FillStackTrace method.

# Extract and display the entire content of CallFrame.cs.
cat runtime/ishtar.vm/runtime/vm/CallFrame.cs

Length of output: 4381

runtime/common/reflection/VeinMethod.cs (1)

62-75: Ensure correct usage of ignoreThis parameter.

The ignoreThis parameter is used to filter arguments. Ensure that it is correctly used and the logic is updated accordingly.

Verification successful

Verified correct usage of ignoreThis parameter.

The ignoreThis parameter is correctly used in the HasCompatibility method to conditionally filter out the this argument. The invocation of this method in VeinClass.cs correctly sets this parameter to true, aligning with the intended behavior.

  • runtime/common/reflection/VeinMethod.cs: Correct implementation of HasCompatibility method.
  • runtime/common/reflection/VeinClass.cs: Appropriate invocation of HasCompatibility method.
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the correctness of the `ignoreThis` parameter usage.

# Test: Search for the `HasCompatibility` method definition. Expect: Correct usage of `ignoreThis` parameter.
rg --type python $'HasCompatibility'

Length of output: 66



Script:

#!/bin/bash
# Description: Verify the correctness of the `ignoreThis` parameter usage in the HasCompatibility method.

# Test: Search for the HasCompatibility method definition in C# files.
rg --type cs 'HasCompatibility'

Length of output: 282



Script:

#!/bin/bash
# Extract the full implementation of the HasCompatibility method in VeinMethod.cs
rg --type cs --context 10 'public bool HasCompatibility' runtime/common/reflection/VeinMethod.cs

# Extract the context of the HasCompatibility method invocation in VeinClass.cs
rg --type cs --context 10 'HasCompatibility' runtime/common/reflection/VeinClass.cs

Length of output: 1850

runtime/common/reflection/VeinTypeCode.cs (1)

32-32: Ensure proper handling of TYPE_NULL.

The new TYPE_NULL enum value should be handled appropriately throughout the codebase. Verify that all necessary checks and operations for TYPE_NULL are implemented.

Verification successful

Proper handling of TYPE_NULL verified.

The new TYPE_NULL enum value is appropriately handled in the codebase, particularly in VirtualMachine.cs and IshtarMarshal.cs.

  • runtime/ishtar.vm/VirtualMachine.cs
  • runtime/ishtar.vm/runtime/IshtarMarshal.cs
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the handling of TYPE_NULL in the codebase.

# Test: Search for usage of TYPE_NULL. Expect: Proper handling in relevant code sections.
rg --type-add 'csharp' --type csharp 'TYPE_NULL'

Length of output: 106



Script:

#!/bin/bash
# Description: Verify the handling of TYPE_NULL in the codebase.

# Test: Search for usage of TYPE_NULL. Expect: Proper handling in relevant code sections.
rg --type-add 'csharp:*.cs' --type csharp 'TYPE_NULL'

Length of output: 1015

runtime/ishtar.generator/GeneratorContext.cs (4)

189-190: LGTM!

The addition of VeinArgumentRef.THIS_ARGUMENT and other arguments in the constructor method seems appropriate.


213-218: LGTM!

The logic for checking hasNotThis and loading arguments appears correct.


221-223: LGTM!

The emitted instructions for loading arguments appear correct.


225-225: LGTM!

The emitted instructions for loading this and calling CALL_SP appear correct.

runtime/common/reflection/VeinClass.cs (1)

124-124: LGTM!

The inclusion of the ignoreThis parameter in the compatibility check seems appropriate.

runtime/ishtar.generator/generators/operators.cs (2)

286-286: LGTM!

The removal of the LDNULL instruction is appropriate as it was likely redundant.


290-293: LGTM!

The inclusion of the function type, raw type, and object type as arguments in the constructor method seems appropriate.

runtime/ishtar.vm/runtime/IshtarMarshal.cs (2)

308-308: LGTM!

The addition of handling null inputs by returning a stackval with type = TYPE_NULL improves null safety.


371-372: LGTM!

The addition of NotImplementedException in the default case ensures that unhandled cases are flagged appropriately.

runtime/ishtar.vm/runtime/gc/IshtarGC.cs (2)

163-175: LGTM!

The AllocRawValue function correctly handles memory allocation and updates GC statistics.


221-227: LGTM!

The FreeRawValue function correctly handles memory deallocation and updates GC statistics.

runtime/ishtar.vm/VirtualMachine.cs (5)

257-266: Verify the necessity of DisableValidationInvocationArgs flag and validation logic.

Ensure that the DisableValidationInvocationArgs flag is necessary and that the argument validation logic is correctly implemented. This flag might be used for performance optimization or specific scenarios where validation is not required.


290-291: LGTM!

The getStackLen function is straightforward and correctly calculates the stack length.


321-327: LGTM!

The ForceThrow function correctly handles exceptions by allocating a new object and setting the stack pointer type to TYPE_CLASS.


588-598: LGTM!

The logic correctly handles null values and boxes values when setting fields.


Line range hint 609-630: LGTM!

The logic correctly handles null values and validates objects before loading fields.

Comment on lines +171 to +172
InsertDebugData(new((ulong)sizeof(rawval),
nameof(AllocValue), (nint)p));
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix the debug info naming.

The debug info name should be AllocRawValue instead of AllocValue.

-                nameof(AllocValue), (nint)p));
+                nameof(AllocRawValue), (nint)p));
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
InsertDebugData(new((ulong)sizeof(rawval),
nameof(AllocValue), (nint)p));
InsertDebugData(new((ulong)sizeof(rawval),
nameof(AllocRawValue), (nint)p));

Comment on lines +713 to +744
case LDFN:
{
++ip;
var tokenIdx = *ip;
var owner = readTypeName(*++ip, _module, invocation);
var method = GetMethod(tokenIdx, owner, _module, invocation);
++ip;

var raw = GC.AllocRawValue(invocation); // TODO destroy

raw->type = VeinRawCode.ISHTAR_METHOD;
raw->data.m = method;

sp->type = TYPE_RAW;
sp->data.p = (nint)raw;
++sp;
} break;
case CALL_SP:
{
++ip;
sp--;

if (sp->type == TYPE_NULL)
{
ForceThrow(KnowTypes.NullPointerException(invocation));
goto exception_handle;
}

var method = *sp;
var raw = (rawval*)method.data.p;
var sw = raw->type;
} break;
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the unused local variable sw.

The local variable sw in the CALL_SP opcode is unused and should be removed to clean up the code.

- var sw = raw->type;
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
case LDFN:
{
++ip;
var tokenIdx = *ip;
var owner = readTypeName(*++ip, _module, invocation);
var method = GetMethod(tokenIdx, owner, _module, invocation);
++ip;
var raw = GC.AllocRawValue(invocation); // TODO destroy
raw->type = VeinRawCode.ISHTAR_METHOD;
raw->data.m = method;
sp->type = TYPE_RAW;
sp->data.p = (nint)raw;
++sp;
} break;
case CALL_SP:
{
++ip;
sp--;
if (sp->type == TYPE_NULL)
{
ForceThrow(KnowTypes.NullPointerException(invocation));
goto exception_handle;
}
var method = *sp;
var raw = (rawval*)method.data.p;
var sw = raw->type;
} break;
case LDFN:
{
++ip;
var tokenIdx = *ip;
var owner = readTypeName(*++ip, _module, invocation);
var method = GetMethod(tokenIdx, owner, _module, invocation);
++ip;
var raw = GC.AllocRawValue(invocation); // TODO destroy
raw->type = VeinRawCode.ISHTAR_METHOD;
raw->data.m = method;
sp->type = TYPE_RAW;
sp->data.p = (nint)raw;
++sp;
} break;
case CALL_SP:
{
++ip;
sp--;
if (sp->type == TYPE_NULL)
{
ForceThrow(KnowTypes.NullPointerException(invocation));
goto exception_handle;
}
var method = *sp;
var raw = (rawval*)method.data.p;
} break;
Tools
GitHub Check: Codacy Static Code Analysis

[notice] 743-743: runtime/ishtar.vm/VirtualMachine.cs#L743
Remove the unused local variable 'sw'.

Comment on lines +202 to +205
ctorGen.Emit(OpCodes.STF, ptrRef); // this.ptrRef = ref;
ctorGen.Emit(OpCodes.LDARG_2); // load scope
ctorGen.Emit(OpCodes.LDARG_0); // load this
ctorGen.Emit(OpCodes.STF, scope); // this.scope = scope;
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove commented-out code.

The commented-out code should be removed to maintain code cleanliness.

- // this.ptrRef = ref;
- // this.scope = scope;
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ctorGen.Emit(OpCodes.STF, ptrRef); // this.ptrRef = ref;
ctorGen.Emit(OpCodes.LDARG_2); // load scope
ctorGen.Emit(OpCodes.LDARG_0); // load this
ctorGen.Emit(OpCodes.STF, scope); // this.scope = scope;
ctorGen.Emit(OpCodes.STF, ptrRef);
ctorGen.Emit(OpCodes.LDARG_2); // load scope
ctorGen.Emit(OpCodes.LDARG_0); // load this
ctorGen.Emit(OpCodes.STF, scope);
Tools
GitHub Check: Codacy Static Code Analysis

[warning] 202-202: runtime/ishtar.generator/GeneratorContext.cs#L202
Remove this commented out code.


[warning] 205-205: runtime/ishtar.generator/GeneratorContext.cs#L205
Remove this commented out code.

Comment on lines +21 to +25
public unsafe struct rawval
{
public rawval_union data;
public VeinRawCode type;

Copy link
Contributor

Choose a reason for hiding this comment

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

Implement IEquatable<T> for rawval.

Implement IEquatable<T> in value type rawval to ensure proper equality checks.

+ public struct rawval : IEquatable<rawval>
+ {
+     public bool Equals(rawval other)
+     {
+         return data.Equals(other.data) && type == other.type;
+     }
+ 
+     public override bool Equals(object obj)
+     {
+         return obj is rawval other && Equals(other);
+     }
+ 
+     public override int GetHashCode()
+     {
+         return HashCode.Combine(data, type);
+     }
+ }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public unsafe struct rawval
{
public rawval_union data;
public VeinRawCode type;
public unsafe struct rawval : IEquatable<rawval>
{
public rawval_union data;
public VeinRawCode type;
public bool Equals(rawval other)
{
return data.Equals(other.data) && type == other.type;
}
public override bool Equals(object obj)
{
return obj is rawval other && Equals(other);
}
public override int GetHashCode()
{
return HashCode.Combine(data, type);
}
}
Tools
GitHub Check: Codacy Static Code Analysis

[warning] 21-21: runtime/ishtar.vm/stackval.cs#L21
Implement 'IEquatable' in value type 'rawval'.

GitHub Check: build_all (ubuntu-latest, linux-x64, true)

[warning] 21-21:
The type name 'rawval' only contains lower-cased ascii characters. Such names may become reserved for the language.

GitHub Check: build_all (macos-latest, osx-x64, false)

[warning] 21-21:
The type name 'rawval' only contains lower-cased ascii characters. Such names may become reserved for the language.

Comment on lines +9 to +13
public unsafe struct rawval_union
{
[FieldOffset(0)] public RuntimeIshtarMethod* m;
[FieldOffset(0)] public RuntimeIshtarClass* c;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Implement IEquatable<T> for rawval_union.

Implement IEquatable<T> in value type rawval_union to ensure proper equality checks.

+ public struct rawval_union : IEquatable<rawval_union>
+ {
+     public bool Equals(rawval_union other)
+     {
+         return m == other.m && c == other.c;
+     }
+ 
+     public override bool Equals(object obj)
+     {
+         return obj is rawval_union other && Equals(other);
+     }
+ 
+     public override int GetHashCode()
+     {
+         return HashCode.Combine(m, c);
+     }
+ }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public unsafe struct rawval_union
{
[FieldOffset(0)] public RuntimeIshtarMethod* m;
[FieldOffset(0)] public RuntimeIshtarClass* c;
}
public unsafe struct rawval_union : IEquatable<rawval_union>
{
[FieldOffset(0)] public RuntimeIshtarMethod* m;
[FieldOffset(0)] public RuntimeIshtarClass* c;
public bool Equals(rawval_union other)
{
return m == other.m && c == other.c;
}
public override bool Equals(object obj)
{
return obj is rawval_union other && Equals(other);
}
public override int GetHashCode()
{
return HashCode.Combine(m, c);
}
}
Tools
GitHub Check: Codacy Static Code Analysis

[warning] 9-9: runtime/ishtar.vm/stackval.cs#L9
Implement 'IEquatable' in value type 'rawval_union'.

Comment on lines +34 to +37
static rawval* allocArray(CallFrame* frame, int size)
=> throw new NotSupportedException();
static void freeArray(CallFrame* frame, rawval* stack, int size)
=> throw new NotSupportedException();
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove unused method parameters.

Remove unused method parameters frame and size to clean up the code.

- static rawval* allocArray(CallFrame* frame, int size)
+ static rawval* allocArray(CallFrame* frame)
- static void freeArray(CallFrame* frame, rawval* stack, int size)
+ static void freeArray(CallFrame* frame, rawval* stack)

Committable suggestion was skipped due to low confidence.

Tools
GitHub Check: Codacy Static Code Analysis

[warning] 34-34: runtime/ishtar.vm/stackval.cs#L34
Remove this unused method parameter 'frame'.


[warning] 36-36: runtime/ishtar.vm/stackval.cs#L36
Remove this unused method parameter 'frame'.

@0xF6 0xF6 merged commit 0997b7e into master Jun 30, 2024
7 of 9 checks passed
@0xF6 0xF6 deleted the fixes/function-method-group-fixes branch June 30, 2024 18:37
@vein-lang vein-lang locked and limited conversation to collaborators Jun 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant