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

rename case statement macro from match to case #16923

Merged
merged 2 commits into from
Feb 8, 2021

Conversation

metagn
Copy link
Collaborator

@metagn metagn commented Feb 3, 2021

closes nim-lang/RFCs#332

Mostly to see what breaks

@metagn
Copy link
Collaborator Author

metagn commented Feb 3, 2021

CI seems fine. Maybe don't merge yet, PRs could be made to fusion etc in anticipation of this first, also could lift from experimental as said in the RFC but PRs for that would have to be after the merge

@Araq Araq merged commit 6a7baff into nim-lang:devel Feb 8, 2021
@timotheecour
Copy link
Member

  • @hlaaftana after running git bisect, this seems to cause a regression, fusion/matching now doesn't work:
when true:
  import fusion/matching
  {.experimental: "caseStmtMacros".}
  case (1, 2):
    of (3, 4), (1, 2):
      discard
    else:
      discard

works before this PR, fails after

Error: attempting to call undeclared routine: 'case'
    case (1, 2):
  • I also notice we don't even have fusion in important_packages; we should add it; having it would've prevented this regression. Note that fusion CI is green but would be red the next time it runs

@metagn
Copy link
Collaborator Author

metagn commented Feb 12, 2021

after running git bisect, this seems to cause a regression, fusion/matching now doesn't work:

The change to rename match to `case` in fusion is in nim-lang/fusion#61, so yes fusion HEAD is broken for now. The change could be made in a separate PR but that PR seemed like it was about to be merged so I commented on it.

@timotheecour
Copy link
Member

timotheecour commented Feb 12, 2021

  • but Pattern matching follow-up PR. fusion#61 does a whole lot of other things, why not have a hot-fix that just renames it to unbreak fusion/matching?
  • also, how about adding nimHasRenamedMatchCase to condsyms.nim so that you can write code that works both before and after this change? (eg, so that 1.4.2 can write caseStmtMacros too)

@metagn
Copy link
Collaborator Author

metagn commented Feb 12, 2021

  • but nim-lang/fusion#61 does a whole lot of other things, why not have a hot-fix that just renames it to unbreak fusion/matching?

  • also, how about adding nimHasRenamedMatchCase to condsyms.nim so that you can write code that works both before and after this change? (eg, so that 1.4.2 can write caseStmtMacros too)

Would be fine with both of these getting done, just initially neither seemed necessary to me.

ardek66 pushed a commit to ardek66/Nim that referenced this pull request Mar 26, 2021
* rename case statement macro from match to `case`

* fix test
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.

Rename case statement macro from match to case
3 participants