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 #76, #82, #83, update docs, more constistent opt <pattern> or <default> implementation #77

Merged
merged 23 commits into from
Jun 3, 2021
Merged

Fix #76, #82, #83, update docs, more constistent opt <pattern> or <default> implementation #77

merged 23 commits into from
Jun 3, 2021

Conversation

haxscramper
Copy link
Contributor

Enabling --gc:{orc,arc} caused macro to behave differently, by somehow mutating immutable access path passed to the match expression generator. I wasn't able to reliably reproduce the bug, but it wasn't related to implementation itself.

Fixed that by explicitly copying path each time it is passed around - usually it is a sequence of 3-4 access elements, and copying only happens during macro evaluation.

Related: #76

@ghost
Copy link

ghost commented Mar 10, 2021

The relevant issue is nim-lang/Nim#17199 by the way so this is a workaround I guess?

@ghost
Copy link

ghost commented Mar 12, 2021

might not be needed if nim-lang/Nim#17348 works and will be merged

@haxscramper
Copy link
Contributor Author

Reverting this is a simple find-replace for fullCopy calls, but it would make it work for current version (because even if nim-lang/Nim#17348 is merged, it would still take quite some time until next release). And it also contains more tests and small fixup for the API (some procs like findItFirstOpt that were meant for internal use were exported)

- ADDED ::
  - implement `opt` for field access
- CHANGED ::
  Before calling check function on nested match element check using
  `isNil` if entry can is correct. This avoids almost unfixable
  nil-based exceptions triggered deeply within pattern.
- CHANGED ::
  - Use more readable, unabreviated field names (cnt -> foundCount,
    fldElems -> fieldElements)
 Compiling /home/runner/work/fusion/fusion/fusion/tests/tmatching (from package fusion) using c backend
Error: /home/runner/work/fusion/fusion/nim/lib/std/private/underscored_calls.nim(43, 21) Error: 'nnkArgList' should be: 'nnkArglist'
       Tip: 5 messages have been suppressed, use --verbose to show them.
@haxscramper
Copy link
Contributor Author

This PR is done - aside from adding missing support for opt pattern for fields it does not implement any new functionality, only fixes some documentation inconsistencies and fixes #82 and #76

@haxscramper
Copy link
Contributor Author

Fix #83

@haxscramper haxscramper changed the title Fix #76 Fix #76, #83, update docs, more constistent opt <pattern> or <default> implementation Apr 21, 2021
@haxscramper haxscramper changed the title Fix #76, #83, update docs, more constistent opt <pattern> or <default> implementation Fix #76, #82, #83, update docs, more constistent opt <pattern> or <default> implementation Apr 21, 2021
@haxscramper
Copy link
Contributor Author

CI failure unrelated to fusion/matching implementation - linux-i386 (version-1-2) test for fusion CI fails because "could not load: libpcre.so(.3|.1|)"

@haxscramper
Copy link
Contributor Author

CI failure unrelated - all the failures happened on "install dependencies" stage.

@Araq Araq merged commit 727e15f into nim-lang:master Jun 3, 2021
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.

None yet

2 participants