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

remove all instances of: raises: [Defect] #16724

Closed

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Jan 14, 2021

rationale 1

procs should be able to raise a Defect by default. Future work can add ability to specify that a proc does not (recursively) generate a Defect. This PR doesn't contradict such design.

rationale 2

compiler (correctly) gives: Hint: 'fn' cannot raise 'Defect' [XCannotRaiseY] here:

when true:
  proc fn*(input: string) {.raises: [Defect].} =
    if input == "asdfadfadf1": raise newException(AssertionDefect, "asdf")
    if input == "asdfadfadf2": doAssert false
    if input == "asdfadfadf3": (var a = 500; var b = a.int8)
  fn("bar")

rationale 3

this doesn't give any warnings/errors

when true:
  proc fn*(input: string) {.raises: [].} =
    if input == "asdfadfadf1": raise newException(AssertionDefect, "asdf")
    if input == "asdfadfadf2": doAssert false
    if input == "asdfadfadf3": (var a = 500; var b = a.int8)
  fn("bar")

rationale 4

nim doc generates: proc fn(input: string) {.raises: [], tags: [].}

  proc fn*(input: string) =
    if input == "asdfadfadf1": raise newException(AssertionDefect, "asdf")
    if input == "asdfadfadf2": doAssert false
    if input == "asdfadfadf3": (var a = 500; var b = a.int8)
  fn("bar")

note

this holds true regardless of --panics:on|off

TODO

  • this seems to fail because of bootstrap, eg:
$nim_1402_X c --lib:lib koch # works
$nim_csources_local_X c --lib:lib koch # fails: options.nim(176, 5) Error: can raise an unlisted exception: ref UnpackDefect
$nim_120_X c --lib:lib koch # ditto
  • simplify assert/doAssert: the cast isn't needed anymore since 1.4.0:
proc failedAssertImpl*(msg: string) {.raises: [], tags: [].} =
  # trick the compiler to not list ``AssertionDefect`` when called
  # by ``assert``.
  type Hide = proc (msg: string) {.noinline, raises: [], noSideEffect,
                                    tags: [].}
  cast[Hide](raiseAssert)(msg)

links

@timotheecour timotheecour mentioned this pull request Jan 14, 2021
2 tasks
@Araq
Copy link
Member

Araq commented Jan 14, 2021

Off topic: New csources are on hold until my M1 machine arrives which will be by the end of January.

@timotheecour
Copy link
Member Author

timotheecour commented Jun 28, 2021

this PR has to wait until csources_v1 is updated to >= 1.4.0 (which is now possible to do without breaking client code thanks to recent improvements in build_all.sh, CI + related logic)
refs timotheecour#251

@stale
Copy link

stale bot commented Jul 7, 2022

This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions.

@stale stale bot added the stale Staled PR/issues; remove the label after fixing them label Jul 7, 2022
@Araq
Copy link
Member

Araq commented Jul 7, 2022

Outdated and of minor importance.

@Araq Araq closed this Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Postponed stale Staled PR/issues; remove the label after fixing them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants