Skip to content

fix build conditions - #2111

Closed
viceice wants to merge 1 commit into
sebastienros:mainfrom
viceice:patch-1
Closed

viceice wants to merge 1 commit into
sebastienros:mainfrom
viceice:patch-1

Conversation

@viceice

@viceice viceice commented Jun 2, 2025

Copy link
Copy Markdown
Contributor

The current conditions are wrong and don't work as intended

$([MSBuild]::IsTargetFrameworkCompatible('net8.0', 'net7.0')) => false
$([MSBuild]::IsTargetFrameworkCompatible('net8.0', 'net6.0')) => false

That's why new Float16Array(2) throws on net8.0

Assert.That(caughtException, expression)
  Expected: <Jint.Runtime.JavaScriptException>
  But was:  <System.NotSupportedException: cannot handle element type
   at Jint.Native.Object.ObjectInstance.ToObject(ObjectTraverseStack stack)
   at Jint.Native.Object.ObjectInstance.ToObject()
   at Jint.Runtime.Interop.InteropHelper.CalculateMethodParameterScore(Engine engine, ParameterInfo parameter, JsValue parameterValue)
   at Jint.Runtime.Interop.InteropHelper.CalculateMethodScore(Engine engine, MethodDescriptor method, JsValue[] arguments)
   at Jint.Runtime.Interop.InteropHelper.FindBestMatch(Engine engine, MethodDescriptor[] methods, Func`2 argumentProvider)+MoveNext()
   at Jint.Runtime.Interop.MethodInfoFunction.Call(JsValue thisObject, JsValue[] jsArguments)
   at Jint.Runtime.Interpreter.Expressions.JintCallExpression.EvaluateInternal(EvaluationContext context)
   at Jint.Runtime.Interpreter.Expressions.JintExpression.GetValue(EvaluationContext context)
   at Jint.Runtime.Interpreter.Statements.JintExpressionStatement.ExecuteInternal(EvaluationContext context)
   at Jint.Engine.ScriptEvaluation(ScriptRecord scriptRecord, ParserOptions parserOptions)
   at Jint.Engine.<>c__DisplayClass99_0.<Execute>b__0()
   at Jint.Engine.ExecuteWithConstraints[T](Boolean strict, Func`1 callback)
   at Jint.Engine.Execute(Prepared`1& preparedScript)
   at Jint.Engine.Evaluate(Prepared`1& preparedScript)
   at Jint.Engine.Evaluate(String code, String source)

@lahma

lahma commented Jun 2, 2025

Copy link
Copy Markdown
Collaborator

I'm not sure if this is the change we want. Generally we want to to check that it's at least net6.0 or similar. This change would suggest these functions don't work and we would need always check '$(TargetFramework)' == 'net8.0'

@viceice

viceice commented Jun 3, 2025

Copy link
Copy Markdown
Contributor Author

maybe it's better to use a built-in conditional.

NET6_0_OR_GREATER

https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/preprocessor-directives#conditional-compilation

@lahma

lahma commented Jun 3, 2025

Copy link
Copy Markdown
Collaborator

I believe the original was from Microsoft documentation. Can you specify the actual problem?

@viceice

viceice commented Jun 3, 2025

Copy link
Copy Markdown
Contributor Author

new Float16Array(2) throws on net8.0 because jint is compiled without Half support.

@lahma

lahma commented Jun 3, 2025

Copy link
Copy Markdown
Collaborator

If I change Jint.Tests.Test262 to target net8.0 all the tests pass (like they do on net9.0). These tests should also cover the Half part. So what am I missing here...

@viceice

viceice commented Jun 5, 2025

Copy link
Copy Markdown
Contributor Author

ok, will try to create a minimal reproduction

@viceice

viceice commented Jun 10, 2025 •

Copy link
Copy Markdown
Contributor Author

Can reproduce it:

diff --git a/Jint.Tests/Runtime/TypedArrayInteropTests.cs b/Jint.Tests/Runtime/TypedArrayInteropTests.cs
index ce0a4a216..ae49261ac 100644
--- a/Jint.Tests/Runtime/TypedArrayInteropTests.cs
+++ b/Jint.Tests/Runtime/TypedArrayInteropTests.cs
@@ -1,3 +1,5 @@
+using Jint.Native;
+
 namespace Jint.Tests.Runtime;

 public class TypedArrayInteropTests
@@ -133,6 +135,10 @@ public class TypedArrayInteropTests
             var fromEngine = engine.GetValue("testSubject");
             Assert.True(fromEngine.IsFloat16Array());
             Assert.Equal(source, fromEngine.AsFloat16Array());
+
+            engine.SetValue("testFunc", new Func<JsTypedArray, JsTypedArray>(v => v));
+
+            Assert.Equal(source, engine.Evaluate("testFunc(testSubject)").AsFloat16Array());
         }
 #endif

Test output

 Jint.Tests.Runtime.TypedArrayInteropTests.CanInteropWithFloat16
   Source: TypedArrayInteropTests.cs line 127
   Duration: 133 ms

  Message: 
System.NotSupportedException : cannot handle element type

  Stack Trace: 
ObjectInstance.ToObject(ObjectTraverseStack stack) line 1058
ObjectInstance.ToObject() line 962
DelegateWrapper.Call(JsValue thisObject, JsValue[] arguments) line 79
JintCallExpression.EvaluateInternal(EvaluationContext context) line 142
JintExpression.GetValue(EvaluationContext context) line 25
JintExpressionStatement.ExecuteInternal(EvaluationContext context) line 24
Engine.ScriptEvaluation(ScriptRecord scriptRecord, ParserOptions parserOptions) line 446
<>c__DisplayClass99_0.<Execute>b__0() line 413
Engine.ExecuteWithConstraints[T](Boolean strict, Func`1 callback) line 819
Engine.Execute(Prepared`1& preparedScript) line 413
Engine.Evaluate(Prepared`1& preparedScript) line 373
Engine.Evaluate(String code, String source) line 350
TypedArrayInteropTests.CanInteropWithFloat16() line 141
RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

@lahma

lahma commented Jun 20, 2025

Copy link
Copy Markdown
Collaborator

I believe the actual issue was fixed with #2121

@viceice viceice closed this Jun 22, 2025
@viceice

viceice commented Jun 22, 2025

Copy link
Copy Markdown
Contributor Author

I believe the actual issue was fixed with #2121

looks good ❤️

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.

2 participants