Skip to content
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

Fix for Nim 2.0.0 - 2.0.4 #35

Merged
merged 5 commits into from
Sep 16, 2024
Merged

Conversation

serjepatoff
Copy link
Contributor

@serjepatoff serjepatoff commented Sep 16, 2024

  1. Generated infix C++ expressions are verbatim text, brutal violations of grouping and precedence were observed under Nim 2.x!!! For example, Nim codegen tried to negate "x == y" as "!x == y". That's why I insulated all occurences with ().

  2. Araq recommended to avoid construction of wrapped STL objects in top-level scope (https://forum.nim-lang.org/t/12518). Fixed unit tests to follow this recommendation.

After performing steps (1,2) cppstl builds and passes all tests when compiled with Nim 2.0.0, 2.0.2 and 2.0.4

PS. It still fails on latest Nim 2.0.8 release because of C++ compile-time errors related to unary 'operator*', but this is unrelated issue.

@Clonkk Clonkk changed the base branch from master to fix-destructor September 16, 2024 08:46
@Clonkk
Copy link
Owner

Clonkk commented Sep 16, 2024

Hi, thanks for your PR.

I already had a bigger PR in the work that fixed the global scope issue. I'll rebase your PR on top the existing one and fix the conflict once it's merged.

@Clonkk Clonkk added the enhancement New feature or request label Sep 16, 2024
@Clonkk Clonkk self-assigned this Sep 16, 2024
@Clonkk Clonkk deleted the branch Clonkk:master September 16, 2024 09:29
@Clonkk Clonkk closed this Sep 16, 2024
@Clonkk Clonkk reopened this Sep 16, 2024
@Clonkk Clonkk changed the base branch from fix-destructor to master September 16, 2024 09:30
@Clonkk Clonkk merged commit 268dff3 into Clonkk:master Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants