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

write system.abs in nim, not magic #485

Draft
wants to merge 5 commits into
base: devel
Choose a base branch
from
Draft

Conversation

timotheecour
Copy link
Owner

@timotheecour timotheecour commented Dec 27, 2020

refs #476 (comment)

subtlety

before PR, nim r main does't raise
after PR, nim r main raises OverflowDefect because abs is now a regular proc.
note, if i change abs to a template, this would preserve pre-existing behavior, ie not raise here.

when true:
  {.push overflowChecks: off.}
  proc main()=
    let a = abs(int64.low)
  main()
  {.pop.}
template abs*[T: SomeSignedInt](x: T): T =
  let x2 = x
  if x2 < 0: -x2 else: x2

vs:

func abs*[T: SomeSignedInt](x: T): T {.inline.} =
  if x < 0: -x else: x

note

if we want to make an inner proc (eg abs) be affected by a pragma (eg overflowChecks:off), there are 2 ways:

  • use a template (simplest but in the general case might not be desirable in all cases)
  • use a proc (generic or not), but make it implicitly generic on the "environment", in this case the pragma overflowChecks:off, so that 2 distinct procs could end up being instantiated in the same program: one with overflowChecks:on and one with overflowChecks:off

note 2

global flags -d:danger aren't affected by this, this is just regarding semantics for a local {.push overflowChecks: off.}

note 3

maybe implicitly generic idea (depending on "environment") could be used in other contexts, eg to generate 2 distinct procs depending on whether we're in vm or not (eg for improving nimvm semantics)

@timotheecour timotheecour force-pushed the pr_abs_nonmagic branch 2 times, most recently from f7b4efa to 2bc9a58 Compare December 27, 2020 09:54
@timotheecour timotheecour changed the base branch from devel to araq-async-refactoring December 27, 2020 10:02
@timotheecour timotheecour changed the base branch from araq-async-refactoring to devel December 27, 2020 10:02
@ringabout
Copy link
Collaborator

@timotheecour
what's the state of this PR?

We have some other stuffs which should be implemented as magics such as isTopLevel, getProcname.

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.

2 participants