-
Notifications
You must be signed in to change notification settings - Fork 102
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
Update dev-pack with last neo changes #146
Conversation
tests/Neo.Compiler.MSIL.UnitTests/Neo.Compiler.MSIL.UnitTests.csproj
Outdated
Show resolved
Hide resolved
tests/Neo.SmartContract.Framework.UnitTests/Neo.SmartContract.Framework.UnitTests.csproj
Outdated
Show resolved
Hide resolved
Need to fix Travis too. Or do we move to Github Actions directly? |
@erikzhang Please, Could you take a look to neo-project/neo#1330 ? |
…Framework.UnitTests.csproj Co-Authored-By: Erik Zhang <erik@neo.org>
…csproj Co-Authored-By: Erik Zhang <erik@neo.org>
Co-Authored-By: Erik Zhang <erik@neo.org>
I think that we should move to github actions directly. What do you prefer? |
|
||
<PropertyGroup> | ||
<Copyright>2015-2019 The Neo Project</Copyright> | ||
<AssemblyTitle>Neo.Compiler.MSIL</AssemblyTitle> | ||
<Version>2.4.1</Version> | ||
<Authors>The Neo Project</Authors> | ||
<TargetFrameworks>netcoreapp2.0;netstandard2.0</TargetFrameworks> | ||
<TargetFrameworks>netcoreapp3.1;netstandard2.1</TargetFrameworks> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we supporting netstandard2.1 here. This is a CLI tool, it should just have the single 3.1 target framework
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it means that it is a library too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need it to be a library too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/neo-project/neo-compiler/pull/147#issue-252914432
This was the PR that turned neon into a library at the time. Maybe some developers have such a need. I do not know.
var testengine = new TestEngine(); | ||
testengine.AddEntryScript("./TestClasses/Contract_shift.cs"); | ||
testengine.ScriptEntry.DumpNEF(); | ||
var result = testengine.ExecuteTestCaseStandard("testfunc"); | ||
ApplicationEngine.Notify -= method; | ||
|
||
CollectionAssert.AreEqual(new BigInteger[] { 16, 17179869184, 4, 0 }, list); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lightszero please take a look this, with int, we have a different result than BigInteger. it's this expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is only BigInteger
in NeoVM, so currently it can only be handled like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But integer result should be the same, because it should be converted to BigInteger. It's strange
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. We should treat the behavior of negative shifts as undefined and disallow it.
Require neo-project/neo-vm#250 neo-project/neo#1331
Issue found neo-project/neo#1330