Ph0enixKM
Remove sed for condition
Ref: #366
In this way we are configuring the scale we want to approximate.
Right now some tests doens't work as we have again issues on bash generation:
function foo__0_v0 {
local a=$1
local b=$2
eval "${a}=$(
eval "echo "scale=0
${!a}/${b}" | bc -l"
)"
}
Instead it should be:
function foo__0_v0 {
local a=$1
local b=$2
eval "${a}=$(
eval "echo \"scale=0
${!a}/${b}\" | bc -l"
)"
}
As you can see the eco inside the second eval is not escaped. I guess the issue is there https://github.com/amber-lang/amber/blob/master/src/modules/shorthand/add.rs#L68
But it is a similar issue of https://github.com/amber-lang/amber/pull/376 with missing/wrong escaping.
@Mte90 but the scale parameter gets rid of the fraction part in the number
@Mte90 but the
scaleparameter gets rid of the fraction part in the number
If you see with the tests everything works in this way. We have issues in some cases because:
#!/usr/bin/env bash
# Written in [Amber](https://amber-lang.com/)
# version: 0.3.4-alpha
# date: 2024-08-09 11:26:27
function foo__0_v0 {
local a=$1
local b=$2
eval "${a}=$(
eval "echo "scale=0
${!a}/${b}" | bc -l"
)"
}
__0_a=15
foo__0_v0 __0_a 3
__AF_foo0_v0__9_1="$__AF_foo0_v0"
echo "$__AF_foo0_v0__9_1" >/dev/null 2>&1
echo ${__0_a}
The echo command is not escaped rightly and the bash script fails. That is part of the other PR as documented. If some cases are not covered for fraction we have to create more tests.
@Mte90 there are some tests that were added but are not on this branch. Can you git pull origin master?
So right now there are all the tests, the problem is that there is a wrong escaping of variables in the generated bash so doesn't work.
I wrote about it on https://github.com/amber-lang/amber/pull/381#issuecomment-2277546441
@Ph0enixKM yeah I wrote on https://github.com/amber-lang/amber/pull/381#issuecomment-2360407534 why there are issues that depends on another bug
#!/usr/bin/env bash # Written in [Amber](https://amber-lang.com/) # version: 0.3.4-alpha # date: 2024-08-09 11:26:27 function foo__0_v0 { local a=$1 local b=$2 eval "${a}=$( eval "echo "scale=0 ${!a}/${b}" | bc -l" )" } __0_a=15 foo__0_v0 __0_a 3 __AF_foo0_v0__9_1="$__AF_foo0_v0" echo "$__AF_foo0_v0__9_1" >/dev/null 2>&1 echo ${__0_a}
@mte90 Can you provide the Amber code that you used to generate this Bash code? Without it, it'll be hard to decipher what went wrong.
If you try to compile the ./src/tests/validity/variable_ref_div_arithmetic_num.ab test you can see the escaping issue.
I improved the code, now the sed trailing zero there is only for the division in this way the code generated is more clean.
fun foo(ref a) {
a += 12
}
let a = 24
foo(a)
echo a
foo__0_v0() {
local a=$1
local b=$2
eval "${a}=$(
eval "echo "scale=3
${!a}/${b}" | bc -l| sed "/\./ s/\.\{0,1\}0\{1,\}$//""
)"
}
__0_a=15
foo__0_v0 __0_a 3
__AF_foo0_v0__9_1="$__AF_foo0_v0"
echo "$__AF_foo0_v0__9_1" >/dev/null 2>&1
echo ${__0_a}
This PR has various issues.
The big one that doesn't let me to finish and do all the tests and verify everything is that the bash generated has errors in the escaping as I wrote here https://github.com/amber-lang/amber/pull/381#issuecomment-2385445358
Until that situation is not fixed in Amber itself I can't do all the checks that are need to move on.
@Mte90 What does this PR actually do? Do you want to remove the usage of sed inside of relation operators like >, <, >=, <=, ==, != ? There was no issue created to explain the problem that this PR was trying to solve. Could you explain it here?
After reading the description I see that you want to fix some escaping problems in Bash. I believe that would be better solved when we implement the TranslationModules
The plan of this PR is to remove the sed usage to escape strings to number as can be optimized.
Right now as it is the PR it is needed sed only for the modulo operation so it means that in a lot of operation the Bash generated is better. For the escaping issue yeah is a long story.
@Mte90 I think that this PR might be closed once we implement the new numeric system (one that does not use bc)