If statement for Iulia / Inline Assembly by chriseth · Pull Request #3220 · argotorg/solidity

chriseth · 2017-11-21T12:41:05Z

This will be much more useful in the ABI decoder (only the encoder is part of develop as of now).

axic · 2017-11-21T14:33:41Z

libjulia/backends/evm/EVMCodeTransform.cpp

+	AbstractAssembly::LabelID end = m_assembly.newLabelId();
+	m_assembly.appendJumpToIf(end);
+	(*this)(_if.body);
+	m_assembly.setSourceLocation(_if.location);


Shouldn't the sourcelocation be before the body or even the condition?

Hm, the source location of the label is correct, but what is wrong is the source location of the ISZERO and the jump above.

Will you look into this?

axic · 2017-11-21T14:34:48Z

I think this is missing the evm1.5 codegen part?

chriseth · 2017-11-21T15:54:21Z

No, AbstractAssembly handles both of them.

axic · 2017-11-22T02:43:47Z

No, AbstractAssembly handles both of them.

Never mind, it is EVMCodeTransform and the if code has no need for evm15 specific stuff.

axic

Please include the if statement in the formal spec

axic · 2017-11-22T13:51:40Z

It is lacking tests:

printing test
assemble test
scope tests (if iszero() { let x := 1 } x := 2)
tests in the julia suite

Though it seems for loop isn't covered in the printing test either.

chriseth · 2017-11-22T14:39:04Z

There is and end-to-end test for if - do we have an "assemble test suite"?

axic · 2017-11-22T14:43:49Z

BOOST_AUTO_TEST_SUITE(Analysis) in test/solidity/InlineAssembly

chriseth · 2017-11-22T15:25:57Z

Tests added.

axic · 2017-11-22T15:30:09Z

test/libjulia/Parser.cpp


+BOOST_AUTO_TEST_CASE(if_statement)
+{
+	BOOST_CHECK(successParse("{ if 42:u256 {} }"));


Hmm, shouldn't in Julia this operate on boolean? if true:bool {}. It currently has a hidden behaviour of if iszero(cond) {}

I can change the test, but we do not yet enforce types anyway, right?

No, not yet. It is fine to leave as it is, we'll at least see if the enforcement triggers this failure :)

I see you've updated it, it is fine that way too.

axic · 2017-11-22T17:49:39Z

docs/julia.rst

        G, L, continue
+    E(G, L, <if condition body>: If) =
+        let G0, L0, v = E(G, L, condition)
+        if v is true or non-zero:


This part is about julia, I think that is more strict and only considers if v is true.

chriseth · 2017-11-23T17:52:23Z

Updated.

axic · 2017-11-23T21:43:41Z

Perhaps we're failing zeppelin tests now as they've checking for even warnings:

> zeppelin-solidity@1.3.0 test /tmp/tmp.Cblq5apBSn
> scripts/test.sh
Starting our own testrpc instance
Error parsing /tmp/tmp.Cblq5apBSn/contracts/Bounty.sol: Warning: This is a pre-release compiler version, please do not use it in production.
Compilation failed. See above.

chriseth · 2017-11-23T22:14:39Z

@axic hm, I fear we have to patch that out, we cannot wait for modified zeppelin code when we introduce a new warning.

axic · 2017-11-27T13:58:49Z

Test failures are due to the Zeppelin issue. This should be fine to merge though as everything else works and it only adds the if feature.

chriseth added the in progress label Nov 21, 2017

chriseth requested a review from axic November 21, 2017 12:41

axic reviewed Nov 21, 2017

View reviewed changes

axic force-pushed the IuliaIf branch from 991c39c to e97f9dd Compare November 22, 2017 03:40

axic suggested changes Nov 22, 2017

View reviewed changes

chriseth force-pushed the IuliaIf branch 2 times, most recently from 6f4ac0e to 239e456 Compare November 22, 2017 13:22

axic added the nextrelease label Nov 22, 2017

chriseth added 3 commits November 22, 2017 16:25

If statement for Iulia / inline assembly.

6dbc34e

Use if statement in abi functions.

6ed4e06

Add if statement to Julia specification.

e15918d

chriseth force-pushed the IuliaIf branch from 239e456 to 8585454 Compare November 22, 2017 15:25

Tests.

9232cd2

chriseth force-pushed the IuliaIf branch from 8585454 to 9232cd2 Compare November 22, 2017 15:27

axic reviewed Nov 22, 2017

View reviewed changes

Force condition to be bool in iulia mode.

1d91b65

axic approved these changes Nov 27, 2017

View reviewed changes

chriseth merged commit a1f59cb into develop Nov 27, 2017

chriseth removed in progress labels Nov 27, 2017

axic deleted the IuliaIf branch November 27, 2017 14:05

Conversation

chriseth commented Nov 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

axic commented Nov 21, 2017

Uh oh!

chriseth commented Nov 21, 2017

Uh oh!

axic commented Nov 22, 2017

Uh oh!

axic left a comment

Choose a reason for hiding this comment

Uh oh!

axic commented Nov 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chriseth commented Nov 22, 2017

Uh oh!

axic commented Nov 22, 2017

Uh oh!

chriseth commented Nov 22, 2017

Uh oh!

axic Nov 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chriseth commented Nov 23, 2017

Uh oh!

axic commented Nov 23, 2017

Uh oh!

chriseth commented Nov 23, 2017

Uh oh!

axic commented Nov 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

axic commented Nov 22, 2017 •

edited

Loading

axic Nov 22, 2017 •

edited

Loading