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.

Sign up for GitHub

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

Jump to bottom

tryEval: only provide value on success #2424

Closed
wants to merge 1 commit into from
Closed

tryEval: only provide value on success #2424

wants to merge 1 commit into from

Conversation

lheckemann
Copy link
Member

See #2421

@lheckemann lheckemann mentioned this pull request Sep 14, 2018
Open
@lheckemann
Copy link
Member Author

@edolstra what do you think of this?

@stale
Copy link

stale bot commented Jun 2, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Jun 2, 2021
Synthetica9
Synthetica9 approved these changes Jun 3, 2021
View reviewed changes
Copy link
Member

@Synthetica9 Synthetica9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense! Enables behaviour like

(builtins.tryEval (assert (1 == 2); 3)).value or "foobar"

(currently this gives false, after this change it would be "foobar")

@stale stale bot removed the stale label Jun 3, 2021
@stale
Copy link

stale bot commented Jan 3, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Jan 3, 2022
@Artturin
Copy link
Member

Artturin commented Dec 5, 2022 •
edited 

(builtins.tryEval (assert (1 == 2); 3)).value or "foobar"

can be done like let x = builtins.tryEval (assert (1 == 2); 3); in if x.success then x.value else "foobar" now

before this is merged we'd have to change nixpkgs

$ rg "tryEval.+value"
nixos/maintainers/option-usages.nix
106:          { name = showOption opt.loc; } // builtins.tryEval (strict opt.value));

lib/tests/misc.nix
345:    ( builtins.tryEval (toInt "") == { success = false; value = false; } )
346:    ( builtins.tryEval (toInt "123 123") == { success = false; value = false; } )
347:    ( builtins.tryEval (toInt "0 123") == { success = false; value = false; } )
348:    ( builtins.tryEval (toInt " 0d ") == { success = false; value = false; } )
349:    ( builtins.tryEval (toInt " 1d ") == { success = false; value = false; } )
350:    ( builtins.tryEval (toInt " d0 ") == { success = false; value = false; } )
351:    ( builtins.tryEval (toInt "00") == { success = false; value = false; } )
352:    ( builtins.tryEval (toInt "01") == { success = false; value = false; } )
353:    ( builtins.tryEval (toInt "002") == { success = false; value = false; } )
354:    ( builtins.tryEval (toInt " 002 ") == { success = false; value = false; } )
355:    ( builtins.tryEval (toInt " foo ") == { success = false; value = false; } )
356:    ( builtins.tryEval (toInt " foo 123 ") == { success = false; value = false; } )
357:    ( builtins.tryEval (toInt " foo123 ") == { success = false; value = false; } )
389:    ( builtins.tryEval (toIntBase10 "") == { success = false; value = false; } )
390:    ( builtins.tryEval (toIntBase10 "123 123") == { success = false; value = false; } )
391:    ( builtins.tryEval (toIntBase10 "0 123") == { success = false; value = false; } )
392:    ( builtins.tryEval (toIntBase10 " 0d ") == { success = false; value = false; } )
393:    ( builtins.tryEval (toIntBase10 " 1d ") == { success = false; value = false; } )
394:    ( builtins.tryEval (toIntBase10 " d0 ") == { success = false; value = false; } )
395:    ( builtins.tryEval (toIntBase10 " foo ") == { success = false; value = false; } )
396:    ( builtins.tryEval (toIntBase10 " foo 123 ") == { success = false; value = false; } )
397:    ( builtins.tryEval (toIntBase10 " foo 00123 ") == { success = false; value = false; } )
398:    ( builtins.tryEval (toIntBase10 " foo00123 ") == { success = false; value = false; } )

nixos/modules/config/terminfo.nix
19:    # can be generated with: filter (drv: (builtins.tryEval (drv ? terminfo)).value) (attrValues pkgs)

pkgs/top-level/splice.nix
59:          inherit (builtins.tryEval value0) success value;

pkgs/top-level/impure.nix
10:  try = x: def: let res = builtins.tryEval x; in if res.success then res.value else def;

it'd also be difficult to support old nix versions too so i think this can't be merged

@stale stale bot removed the stale label Dec 5, 2022
@fricklerhandwerk fricklerhandwerk added language The Nix expression language; parser, interpreter, primops, evaluation, etc feature Feature request or proposal labels Mar 3, 2023
@Ericson2314
Copy link
Member

I think this would be a good idea if we had a language versioning story.

@roberth
Copy link
Member

roberth commented Apr 14, 2023

This would make for a good addition to Nixpkgs lib.

Small but breaking changes like this are not worth the cost of breaking old expressions.

a good idea if we had a language versioning story.

Nixpkgs lib is a decent story for "replacing" builtins.
We'd only really need a language version for changing the syntax, or the meaning of operators. Individual builtins, while built in, are not something you'd have to use, so we have little reason to break those.

@lheckemann lheckemann closed this Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Synthetica9 Synthetica9 Synthetica9 approved these changes

Assignees
No one assigned
Labels
feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc
Projects
Nix team
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@lheckemann @Artturin @Ericson2314 @roberth @Synthetica9 @fricklerhandwerk