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

tuple unpacking in const/let/var disallows pragmas #316

Merged
merged 1 commit into from
May 24, 2022

Conversation

saem
Copy link
Collaborator

@saem saem commented May 16, 2022

Summary

tuple unpacking in const/let/var disallows pragmas

let (a {.compileTime.}, b) = (1, 2) and most other pragmas led to nil
pointer exceptions in the compiler, didn't make sense as the example, or
in some other way posed a significant issue. For now they've been made
illegal.

Details

Primary Changes

  • the compiler will show an error message if attempted
  • there tests for these errors
  • the manual.rst has a note added to the tuple unpacking section

Other Changes/Notes:

  • add more compiler trace points, as needed for debugging
  • added trace points for PNode -> TCandidate procs
  • semVarOrLet is much longer and a bit more linear
  • minor formatting touch ups here and there

Future work:

  • semVarOrLet and semConst should likely get merged
  • need to take more advantage of nkError to reduce code

Random Lessons:

  • the compiler is already using lowerings in sem due to ambiguities
  • assignment overloads don't work in const/var/let
  • tuple unpacking potentially evaluates the rhs multiple times

@saem saem force-pushed the saem-cleanup-semvarorlet branch 2 times, most recently from 357bb3d to ce6bdc4 Compare May 23, 2022 22:51
`let (a {.compileTime.}, b) = (1, 2)` and most other pragmas led to nil
pointer exceptions in the compiler, didn't make sense as the example, or
in some other way posed a significant issue. For now they've been made
illegal.

Highlevel Change:
- the compiler will show an error message if attempted
- there tests for these errors
- the manual.rst has a note added to the tuple unpacking section

Other Changes/Notes:
- add more compiler trace points, as needed for debugging
- added trace points for PNode -> TCandidate procs
- `semVarOrLet` is much longer and a bit more linear
- minor formatting touch ups here and there

Future work:
- `semVarOrLet` and `semConst` should likely get merged
- need to take more advantage of nkError to reduce code

Random Lessons:
- the compiler is already using lowerings in sem due to ambiguities
- assignment overloads don't work in const/var/let
- tuple unpacking potentially evaluates the rhs multiple times
@saem saem force-pushed the saem-cleanup-semvarorlet branch from ce6bdc4 to 1d7def2 Compare May 23, 2022 22:56
@saem saem changed the title refactor compiler/sem/semstmts.semVarOrLet tuple unpacking in const/let/var disallows pragmas May 23, 2022
@saem saem marked this pull request as ready for review May 23, 2022 22:59
@saem
Copy link
Collaborator Author

saem commented May 24, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented May 24, 2022

👎 Rejected by PR status

@saem saem closed this May 24, 2022
@saem saem reopened this May 24, 2022
@saem
Copy link
Collaborator Author

saem commented May 24, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented May 24, 2022

Build succeeded:

@bors bors bot merged commit 959e4bd into nim-works:devel May 24, 2022
bors bot added a commit that referenced this pull request Jun 20, 2022
357: compiler: use nkError in sem for import export r=saem a=saem

## Summary
- semExportExceptStmt is reworked like semVarOrLet
- removed deadcode in importer `importForwarded`
- more of importer is now nkError aware

`semVarOrLet` refactor: #316



Co-authored-by: Saem Ghani <[email protected]>
@saem saem deleted the saem-cleanup-semvarorlet branch January 22, 2023 18:42
@saem saem restored the saem-cleanup-semvarorlet branch January 22, 2023 18:42
@saem saem deleted the saem-cleanup-semvarorlet branch January 22, 2023 18:42
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