| 10 Dec 2024 |
 puck | In reply to @piegames:flausch.social
I still don't understand why the Niv error happens, or where the override happens uhhh, so, this is in TVL; default.nix sets a scopedArgs, which readTree uses to scopedImport the files; the sources.nix is then imported thru that | 11:51:36 |
| puck | * uhhh, so, this is in TVL; default.nix sets a scopedArgs, which readTree uses to scopedImport the files; the sources.nix is then imported with scopedImport and those scopedArgs | 11:51:47 |
| puck | * uhhh, so, this is in the TVL repo; default.nix sets a scopedArgs, which readTree uses to scopedImport the files; the sources.nix is then imported with scopedImport and those scopedArgs | 11:51:51 |
| puck | it's a bit of a roundabout non-standard way to go at this :p | 11:52:09 |
 raitobezarius | yeah, i wonder if we should do something about the people who are going to copy the tvl monorepo kit | 11:52:47 |
| puck | interestingly, the code that calls __findFile is never evaluated | 11:53:12 |
 piegames | Yeah I think that is the issue here. The tvix solution only breaks when actually evaluated, while the deprecation check eagerly happens at variable binding time (even for dead code) | 11:54:31 |
| raitobezarius | Can I leave it to you pie to make an answer to the issue with your ideas on how to solve this? | 11:55:38 |
| piegames | I'll answer later | 11:57:29 |
| piegames | But my take is: On the one hand, it only breaks TVIX for a weird use case and there is an escape hatch to disable the error. On the other hand, I mostly care about __sub and __findFile was more a bonus for completeness anyways, so could easily also revert that bit | 12:00:25 |
| piegames | I'll ask pennae in person tomorrow | 12:00:49 |
| piegames | * I'll ask pennae in person tomorrow pennae says "break tvix" | 12:53:11 |
| piegames | Though I see why Flokli's bug report is confused about Niv, the error message really points to the wrong place | 12:53:36 |
| piegames | Fixing this might be tricky and I don't want to sink too much energy into slightly improving the experience of a feature deprecation, but I will at least try to see if I can improve it a bit | 12:54:12 |
| puck | i think it might be worth checking the "will this shadow something" in importScoped and lets? | 12:56:34 |
| piegames | puck: So I made the check on use site instead of declaration site to still allow setting variables with that name which could shadow things as long as they don't | 13:13:49 |
| piegames | Though there are only two sites where this can even happen, let bindings and arguments passed to scopedImport. (Attrsets are out despite the existence of with, because it will never have precedence) | 13:14:49 |
| puck | In reply to @piegames:flausch.social
Though there are only two sites where this can even happen, let bindings and arguments passed to scopedImport. (Attrsets are out despite the existence of with, because it will never have precedence) actually false :D but in a really nasty way | 13:15:18 |
| piegames | oh no 🙈 | 13:15:25 |
| puck | rec { __findFile = _: _: 5; a = <meow>; } | 13:15:32 |
| piegames | Oh noes | 13:15:54 |
| piegames | see, good thing I didn't try the other approach because I would have certainly failed that case | 13:16:09 |
| puck | In reply to @piegames:flausch.social
puck: So I made the check on use site instead of declaration site to still allow setting variables with that name which could shadow things as long as they don't it would've been plausible to warn, i guess | 13:16:40 |
| puck | but yeah, complexity | 13:17:11 |
| puck | the only thought i had is "if an ExprVar would bind to a shadowed var, warn + replace it with an error value" | 13:17:43 |
| puck | * the only thought i had is "if an ExprVar would bind to a shadowed var, print a warning + replace it with an error value" | 13:17:50 |
| piegames | that's an interesting approach | 13:18:14 |
| piegames | it would add some laziness, which in this case would not break the Tvix code | 13:18:25 |
| puck | and then later ban shadowing altogether | 13:18:27 |
| piegames | Though I think breaking Tvix here is okay because the exact change allows them to do exactly what they want to do in the first place but better | 13:19:24 |
