Closed seanyoung closed 2 years ago
Issue Status: 1. Open 2. Started 3. Submitted 4. Done
This issue now has a funding of 400.0 DAI (400.0 USD @ $1.0/DAI) attached to it.
@W3F It looks like gitcoin's github integration bot is malfunctioning again, and didn't seem to post the bounty applicants to this issue and #282. Do you mind taking a look at it?
Ha! @KiChjang you are correct, the bot did not post to the issue. @seanyoung could you have a look at the two applicants and let us know who to approve: https://gitcoin.co/issue/hyperledger-labs/solang/284/100023827
@Web3Foundation please approve @janus thank you
@janus Are you still working on this bounty, or is it ok for others to work on it?
@janus Are you still working on this bounty, or is it ok for others to work on it?
I didn't receive 'approval' message. Please approve it then.
@Web3Foundation Please approve me.
Issue Status: 1. Open 2. Started 3. Submitted 4. Done
Work has been started.
These users each claimed they can complete the work by 265 years, 9 months from now. Please review their action plans below:
1) kichjang has applied to start work _(Funders only: approve worker | reject worker)_.
First, the compiler frontend will need to be extended so that it supports the regex for rationals. Next, a new AST enum variant for rationals will be created, and generated appropriately when the lexer finds a rational token. After that, codegen needs to evaluate and generate the appropriate LLVM bytecode to support rational arithmetic. Finally, we'll need tests to exercise the entire process and make sure rounded rationals gets implicitly cast into integers. 2) janus has been approved to start work.
I am currently participating in a language development. I have done a few toys language development works in the paste. I also participated in a language called Citrine. I like challenges and this task would lead me to study the core codebase of your language and from there participate in future development. The task as stated involved tweaking the tokenizer(assuming it is not already there) , and adding Rational Object and include its evaluation and casting.
I have working knowledge of C and Rust languages.
Learn more on the Gitcoin Issue Details page.
done @janus
done @janus
Thanks
@janus Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
Issue Status: 1. Open 2. Started 3. Submitted 4. Done
@janus due to inactivity, we have escalated this issue to Gitcoin's moderation team. Let us know if you believe this has been done in error!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
@gitcoinbot This is actively worked on....update coming soon.
@janus Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
@seanyoung , @Web3Foundation , @KiChjang
I got the below error message with this command: cargo test
. I have removed my cache.
error: failed to run custom build command for `llvm-sys v100.2.0`
Caused by:
process didn't exit successfully: `/home/kaka/buildtool/job/amhello/gitcoin/solang/target/debug/build/llvm-sys-d6938d4c65a04c4e/build-script-build` (exit code: 101)
--- stdout
cargo:rerun-if-env-changed=LLVM_SYS_100_PREFIX
cargo:rerun-if-env-changed=LLVM_SYS_100_IGNORE_BLACKLIST
cargo:rerun-if-env-changed=LLVM_SYS_100_STRICT_VERSIONING
cargo:rerun-if-env-changed=LLVM_SYS_100_NO_CLEAN_CFLAGS
cargo:rerun-if-env-changed=LLVM_SYS_100_USE_DEBUG_MSVCRT
cargo:rerun-if-env-changed=LLVM_SYS_100_FFI_WORKAROUND
--- stderr
thread 'main' panicked at 'Failed to search PATH for llvm-config: Permission denied (os error 13)', /home/kaka/.cargo/registry/src/github.com-1ecc6299db9ec823/llvm-sys-100.2.0/build.rs:121:23
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...
error: build failed
@seanyoung , @Web3Foundation , @KiChjang
I got the below error message with this command:
cargo test
. I have removed my cache.error: failed to run custom build command for `llvm-sys v100.2.0` Caused by: process didn't exit successfully: `/home/kaka/buildtool/job/amhello/gitcoin/solang/target/debug/build/llvm-sys-d6938d4c65a04c4e/build-script-build` (exit code: 101) --- stdout cargo:rerun-if-env-changed=LLVM_SYS_100_PREFIX cargo:rerun-if-env-changed=LLVM_SYS_100_IGNORE_BLACKLIST cargo:rerun-if-env-changed=LLVM_SYS_100_STRICT_VERSIONING cargo:rerun-if-env-changed=LLVM_SYS_100_NO_CLEAN_CFLAGS cargo:rerun-if-env-changed=LLVM_SYS_100_USE_DEBUG_MSVCRT cargo:rerun-if-env-changed=LLVM_SYS_100_FFI_WORKAROUND --- stderr thread 'main' panicked at 'Failed to search PATH for llvm-config: Permission denied (os error 13)', /home/kaka/.cargo/registry/src/github.com-1ecc6299db9ec823/llvm-sys-100.2.0/build.rs:121:23 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace warning: build failed, waiting for other jobs to finish... error: build failed
Discard the above I have figured it out
@seanyoung Please go through my code and comment. I would want to know if I followed your convention. WIP, https://github.com/hyperledger-labs/solang/pull/384
@janus your code looks fine, thank you.
You do need a "Signed-off-by:" in your commit message.
@janus Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
@seanyoung Can we make a payout to @janus ?
Not yet...I was unwell... I would add up more features this week please
On Wed, 17 Feb 2021, 09:58 W3F, notifications@github.com wrote:
@seanyoung https://github.com/seanyoung Can we make a payout to @janus https://github.com/janus ?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/hyperledger-labs/solang/issues/284#issuecomment-780406833, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABURWLYYRYBPDMXTZXG33S7OAKVANCNFSM4RTZ22JA .
This issue is not really out:
sudo chown -R $(whoami) /home/kaka/.cargo/ This resolved the issue before but not now. I also noticed this: sudo cargo build
error: no override and no default toolchain set
aused by:
process didn't exit successfully: `/home/kaka/buildtool/job/amhello/gitcoin/solang/target/debug/build/llvm-sys-34f8483076ad8ea7/build-script-build` (exit code: 101)
--- stdout
cargo:rerun-if-env-changed=LLVM_SYS_100_PREFIX
cargo:rerun-if-env-changed=LLVM_SYS_100_IGNORE_BLACKLIST
cargo:rerun-if-env-changed=LLVM_SYS_100_STRICT_VERSIONING
cargo:rerun-if-env-changed=LLVM_SYS_100_NO_CLEAN_CFLAGS
cargo:rerun-if-env-changed=LLVM_SYS_100_USE_DEBUG_MSVCRT
cargo:rerun-if-env-changed=LLVM_SYS_100_FFI_WORKAROUND
--- stderr
thread 'main' panicked at 'Failed to search PATH for llvm-config: Permission denied (os error 13)', /home/kaka/.cargo/registry/src/github.com-1ecc6299db9ec823/llvm-sys-100.2.0/build.rs:121:23
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...
Is there a permanent solution to this? I need help
@janus you need to install the llvm libraries as per the documentation: https://solang.readthedocs.io/en/latest/installing.html#installing-llvm-on-linux
error: no override and no default toolchain set
is an issue with rustup. Run rustup default stable
@seanyoung , I have added unit test (lexer) for decimal. Please, check if it is exhaustive.
@janus you need to install the llvm libraries as per the documentation: https://solang.readthedocs.io/en/latest/installing.html#installing-llvm-on-linux
error: no override and no default toolchain set
is an issue with rustup. Runrustup default stable
Thanks. I erroneously deleted llvm file.
@seanyoung , I have added unit test (lexer) for decimal. Please, check if it is exhaustive.
The lexer looks complete, but I haven't tested it.
@seanyoung From the example above , it would only be unsigned, is that correct?
@seanyoung ,
Is solidity.lalrprop
machine generated? If yes, how do I have to add Rational token to it? How is solidity.rs
related to the parser?
I noticed that Type
and Expression
enums are defined in both pt.rs
and ast.rs
.
Where is tokens consumed?
@janus the lexer only lexes positive values, and lexes the minus as a separate token. The resolver (sema) combines the two.
solidity.lalrpop is an LR grammar, very much handwritten. The solidity.rs is generated from this file. See http://lalrpop.github.io/lalrpop/
@janus Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
@seanyoung
I opted to use bigdecimal::BigDecimal
. I could also use bigInt for numerator , bigInt for denominator. But I feel that BigDecimal is a natural fit here. Please review my update and also solidity.lalrpop.
Which command do you use to convert lalrpop file to rs?
@janus cargo build
will generate src/lexer/solidity.rs
if src/lexer/solidity.lalrpop
has changed.
@janus Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
@seanyoung
Please check out bigfloat_to_expression
function. It is a bit sloppy. I need your input.
@janus Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
Issue Status: 1. Open 2. Started 3. Submitted 4. Done
@janus due to inactivity, we have escalated this issue to Gitcoin's moderation team. Let us know if you believe this has been done in error!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
@seanyoung Can we make a payout to @janus? Or what is the status with this?
@Web3Foundation the work is far from complete, unfortunately
@seanyoung I asked for little direction but no response. I am ready to get back if you would just support me by way of answering one or two questions. Thsnks
@janus sorry about that, I missed that.
So I think you're on the right track. Only nitpicks I have is:
bigfloat_to_expression
isn't a great name; might as well call it bigdecimal_to_expression
uint256
or uint32
.rational
crate. This is up to you.I'll keep an eye out so I don't miss anything again!
@janus please let me know if there is anything else I can do for you
@seanyoung Thanks so much. I would get back to coding and if I have issues along the way I would get back to you.
@seanyoung Should Rational
be classified as primitive? That is returning true for is_primitive
.
I have a nasty error with num_rational
, I have contacted the team.
@janus yes, a rational is a primitive in Solidity.
Out of interest, what is the issue you are having with num_rational?
@seanyoung Thanks for the quick response.
There may be something out there I am not following.
BigRational::new(BigInt::from_str(&significand).unwrap(), denominator.mul(exp))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `num_bigint::bigint::BigInt`, found struct `BigInt`
|
= note: perhaps two different versions of crate `num_bigint` are being used?
I think the note:
is likely the point to the issue. You may have a different version num_bigint in your Cargo.toml which differs what the version that num_bigint depends on.
I think the
note:
is likely the point to the issue. You may have a different version num_bigint in your Cargo.toml which differs what the version that num_bigint depends on.
Thanks. I would investigate.
@seanyoung
I got the below. It is strange to me.
$ cargo --version cargo 1.50.0 (f04e7fab7 2021-02-04)
$ rustc --version rustc 1.50.0 (cb75ad5db 2021-02-10)
error[E0658]: or-patterns syntax is experimental
--> src/sema/contracts.rs:1043:13
|
1043 | pt::Visibility::Public(_) | pt::Visibility::External(_),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #54883 <https://github.com/rust-lang/rust/issues/54883> for more information
error[E0658]: or-patterns syntax is experimental
--> src/sema/contracts.rs:1044:13
|
1044 | pt::Visibility::Public(_) | pt::Visibility::External(_)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #54883 <https://github.com/rust-lang/rust/issues/54883> for more information
error[E0658]: or-patterns syntax is experimental
--> src/sema/expression.rs:1329:43
|
1329 | | (Mutability::Nonpayable(_), Mutability::Nonpayable(_) | Mutability::Payable(_))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #54883 <https://github.com/rust-lang/rust/issues/54883> for more information
error[E0658]: or-patterns syntax is experimental
--> src/sema/expression.rs:1331:37
|
1331 | | (Mutability::View(_), Mutability::View(_) | Mutability::Nonpayable(_) | Mutability::Payable(_))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #54883 <https://github.com/rust-lang/rust/issues/54883> for more information
@janus you need to upgrade your rust to at least 1.53.0, sorry about that.
@seanyoung Thanks so much. It worked Please review my code. And point out file I would work on next.
In Solidity, rationals are supported as long as they are compile-time constants. e.g. this is supported:
Also, fractions can be written with scientific notation:
1e-2
or0.001e2
.Fractions can be added/subtracted at compiler time, but fractions cannot be added to integers, like:
This change requires: