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.

Sign up for GitHub

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

Jump to bottom

strictFuncs + views: cannot import pegs #16892

Closed
ee7 opened this issue Jan 31, 2021 · 3 comments · Fixed by #18951
Closed

strictFuncs + views: cannot import pegs #16892

ee7 opened this issue Jan 31, 2021 · 3 comments · Fixed by #18951
Labels
Strict Functions View Types

Comments

@ee7
Copy link
Contributor

ee7 commented Jan 31, 2021 •
edited
Loading

Example

Compile the below with nim c --experimental:strictFuncs --experimental:views

import pegs

Current Output

/nim/lib/pure/pegs.nim(344, 6) Error: 'nonterminal' can have side effects

Expected Output

No error.

Additional Information

Note that the error does not occur with just nim c --experimental:strictFuncs

To "solve" this issue by removing instances of noSideEffect, the minimal number of noSideEffect that must be removed is shown in this commit: ee7@23945aa. I assume that this approach isn't what we want, but tell me otherwise and I'll make the PR for it.

Apart than critbits (see #16873) and pegs, it looks like every stdlib module can be imported when compiling with

--experimental:strictFuncs --experimental:views

See #16878.

Nim version

This issue occurs in:

  • the latest devel commit (c05d1aa)
  • Nim 1.4.2
  • Nim 1.4.0
@timotheecour
Copy link
Member

timotheecour commented Feb 1, 2021 •
edited
Loading

@ee7 how about:

  • close this issue and merge it into strictFuncs: cannot import critbits #16873, and rename that issue to strictFuncs: cannot import X; it's the same underlying reason, no need to open N issues for that; it'll also make tracking easier (use check boxes)
  • ditto with any similar issue created by you or others

@ee7
Copy link
Contributor Author

ee7 commented Feb 1, 2021 •
edited
Loading

@timotheecour I see your argument, but at this moment I think it's cleanest to keep the issues separate.

The issues differ in at least these ways:

  • The critbits issue is a regression since the latest stable release, and so might be resolved more quickly. But import pegs has never worked with strictFuncs + views; the critbits issue can be tagged with "regression", but the pegs one can't.
  • The critbits issue also is easier to "fix" via the approach of func -> proc, or removing noSideEffect.
  • The pegs issue requires --experimental:views.

it'll also make tracking easier (use check boxes)

I was guessing that the critbits issue would be resolved more quickly - I could argue that it's better for a commit to close an issue than to tick a box.

no need to open N issues for that

As far as I can tell, there are only two stdlib modules affected - I hope that N is exactly 2. Furthermore, we can guarantee that N does not increase (due to the current stdlib modules at least) if something like #16878 is merged.

However, if we find any more "cannot import X with strictFuncs/views" then I'm happy to do as you say and have one combined issue.

Does that work for you?

Edit: #16877 was merged, which closed #16873. If we find any new "cannot import X with strictFuncs/views" problems then I'm fine with tracking them in this issue.

@ee7 ee7 mentioned this issue Feb 1, 2021
Merged
ee7 added a commit to ee7/Nim that referenced this issue Feb 1, 2021
@ee7
strictFuncs+views: add test that imports stdlib modules
73df251 
This commit attempts to improve testing of strictFuncs and views, and
prevent regressions like nim-lang#16873 (resolved by 0b01edd).

We previously only explicitly tested strictFuncs and views with a
smaller number of stdlib modules, mostly in:
- tests/effects/tstrict_funcs.nim
- tests/views/tcan_compile_nim.nim

Note that this commit leaves the `pegs` module commented out; it
cannot currently be compiled with `--experimental:views` (see nim-lang#16892).

Note also that this commit is not sufficient to test strictFuncs and
views, but it does detect a subset of problems.
Araq pushed a commit that referenced this issue Feb 3, 2021
@ee7
strictFuncs+views: add test that imports stdlib modules (#16878)
14c61ec 
This commit attempts to improve testing of strictFuncs and views, and
prevent regressions like #16873 (resolved by 0b01edd).

We previously only explicitly tested strictFuncs and views with a
smaller number of stdlib modules, mostly in:
- tests/effects/tstrict_funcs.nim
- tests/views/tcan_compile_nim.nim

Note that this commit leaves the `pegs` module commented out; it
cannot currently be compiled with `--experimental:views` (see #16892).

Note also that this commit is not sufficient to test strictFuncs and
views, but it does detect a subset of problems.
@ghost ghost added the Strict Functions label Feb 15, 2021
ardek66 pushed a commit to ardek66/Nim that referenced this issue Mar 26, 2021
@ee7 @ardek66
strictFuncs+views: add test that imports stdlib modules (nim-lang#16878)
f39f8e1 
This commit attempts to improve testing of strictFuncs and views, and
prevent regressions like nim-lang#16873 (resolved by 0b01edd).

We previously only explicitly tested strictFuncs and views with a
smaller number of stdlib modules, mostly in:
- tests/effects/tstrict_funcs.nim
- tests/views/tcan_compile_nim.nim

Note that this commit leaves the `pegs` module commented out; it
cannot currently be compiled with `--experimental:views` (see nim-lang#16892).

Note also that this commit is not sufficient to test strictFuncs and
views, but it does detect a subset of problems.
@nc-x nc-x mentioned this issue May 27, 2021
Closed
@juancarlospaco
Copy link
Collaborator

This still repro as of today. :(

quantimnot added a commit to quantimnot/Nim that referenced this issue Oct 4, 2021
@quantimnot
Fixed strictFuncs support for std/pegs
5e460cd 
Enabled `std/pegs` in the `strictFuncs` import test.

Fixes nim-lang#18057
Fixes nim-lang#16892
See nim-lang#18111
@quantimnot quantimnot mentioned this issue Oct 4, 2021
Merged
quantimnot added a commit to quantimnot/Nim that referenced this issue Oct 4, 2021
@quantimnot
Fixed strictFuncs support for std/pegs
bff35b6 
Enabled `std/pegs` in the `strictFuncs` import test.

Fixes nim-lang#18057
Fixes nim-lang#16892
See nim-lang#18111
Araq pushed a commit that referenced this issue Oct 7, 2021
@quantimnot
Fixed strictFuncs support for std/pegs (#18951)
19774a7 
* Fixed `strictFuncs` support for `std/pegs`

Enabled `std/pegs` in the `strictFuncs` import test.

Fixes #18057
Fixes #16892
See #18111

* Rebased from `devel`

* Conditionally compile `std/pegs` in `koch`

This is for supporting `csources` bootstrap.

Co-authored-by: quantimnot <quantimnot@users.noreply.github.com>
PMunch pushed a commit to PMunch/Nim that referenced this issue Mar 28, 2022
@quantimnot @PMunch
Fixed strictFuncs support for std/pegs (nim-lang#18951)
52bd0c4 
* Fixed `strictFuncs` support for `std/pegs`

Enabled `std/pegs` in the `strictFuncs` import test.

Fixes nim-lang#18057
Fixes nim-lang#16892
See nim-lang#18111

* Rebased from `devel`

* Conditionally compile `std/pegs` in `koch`

This is for supporting `csources` bootstrap.

Co-authored-by: quantimnot <quantimnot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Strict Functions View Types
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixed strictFuncs support for std/pegs quantimnot/Nim
pegs.nim now compiles with strictFuncs
3 participants
@juancarlospaco @timotheecour @ee7