Skip to content

[FIX] preserve parens around expressions calculating value of argument for a function call #67

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

Merged
merged 2 commits into from
Oct 18, 2022

Conversation

filipsajdak
Copy link
Contributor

The current implementation removes parentheses used in expression that calculates the value of arguments for a function.

cpp2 code:

f: (x:_) = {}

main: () -> int = {
    i := 2;
    f((i+(i+1)*2)/2);
}

When compiled with cppfront we get (I am skipping the boilerplate)

[[nodiscard]] auto main() -> int{
    auto i { 2 }; 
    f( i + i + 1 * 2 / 2);
}

All parentheses inside the function call are gone. (i+(i+1)*2)/2 is not equal i + i + 1 * 2 / 2.

This fix corrects that behavior and preserves these parentheses. The generated code after the fix:

[[nodiscard]] auto main() -> int{
    auto i { 2 }; 
    f((i + (i + 1) * 2) / 2);
}

Close #66

@filipsajdak
Copy link
Contributor Author

This fix breaks

v : std::vector<int> = ( 1, 2, 3 );

cppfront generates

std::vector<int> v { (1, 2, 3) };

which compiles but thanks to the coma operator it will compile by cpp1 compiler to a vector with only 3 value.

Current solution compiles `v : std::vector<int> = ( 1, 2, 3 );` into
`std::vector<int> v { (1, 2, 3) };`

This commit change that behaviour and removes extra parens from
generated code.
@filipsajdak
Copy link
Contributor Author

filipsajdak commented Oct 11, 2022

OK, the above issue is now fixed.

@filipsajdak
Copy link
Contributor Author

Some issue still exists

cpp2 code

ddd := fun(fun("abc").substr(1));

compiles to

auto ddd { fun(CPP2_UFCS(substr, fun(("abc")), 1)) };

@filipsajdak filipsajdak marked this pull request as draft October 11, 2022 01:46
@filipsajdak filipsajdak force-pushed the fsajdak-fix-preserve-parens branch from fbac220 to 935758f Compare October 11, 2022 05:46
@filipsajdak
Copy link
Contributor Author

The issue with additional parens around function arguments is solved.

The issue showed up in the code merged with #18 (I was not able to trigger it on the main branch). I remove that commit from the patch set.

@filipsajdak filipsajdak marked this pull request as ready for review October 11, 2022 05:52
@hsutter
Copy link
Owner

hsutter commented Oct 18, 2022

Good fix -- this actually corrects the output for two existing test cases, one of which is mixed-test-parens.cpp2 that I hadn't noticed had the wrong output. As Bjarne would say: "Eek!" Thanks.

@hsutter hsutter merged commit c511638 into hsutter:main Oct 18, 2022
@filipsajdak filipsajdak deleted the fsajdak-fix-preserve-parens branch October 18, 2022 04:33
Azmah-Bad pushed a commit to Azmah-Bad/cppfront that referenced this pull request Feb 24, 2023
…arens

[FIX] preserve parens around expressions calculating value of argument for a function call
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.

[BUG] parens are not preserved when used for calculating argument value
2 participants