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

unsafeGetAttrPos is not unsafe. tryGetAttrPos? #5557

Open
roberth opened this issue Nov 14, 2021 · 0 comments
Open

unsafeGetAttrPos is not unsafe. tryGetAttrPos? #5557

roberth opened this issue Nov 14, 2021 · 0 comments

Comments

@roberth
Copy link
Member

roberth commented Nov 14, 2021

Describe the bug

Its unsafety is an exaggeration. It cannot guarantee to return a (good?) position for any arbitrary attribute, but that does not make it unsafe.
It could even be said that unsafeGetAttrPos is safer than readFile because it doesn't even throw an exception, but I'd consider them to be in the same class of reliability.

Steps To Reproduce

  1. Read perlPackages: make meta.position point to the right location nixpkgs#114647 (comment)
  2. See that people don't want to use it because it's "unsafe"

Expected behavior

unsafeGetAttrPos causes a crash. I mean, that's what people would expect, but we ought to just fix the name by introducing an alias for the function. What about tryGetAttrPos?

nix-env --version output

Additional context

This is probably "cultural heritage" from Haskell's unsafePerformIO: a function that isn't inherently unsafe, but allows things to be done that shouldn't be possible within the language semantics. It can cause issues through interactions with other parts of the language that rely on referential transparency for memory safety, so the unsafe qualifier is appropriate there. However, it is quite safe to use in predictable circumstances, which I believe leads people to use the same qualifier for functions that are actually safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant