If @amjoseph-nixpkgs were around, they might be able to help understand how these lines ended up in the patch. My best guess is some path dependent reason that went away with a change to the implementation.
Short answer: LGTM
Medium answer: I’m on the road right now and monkeying around with git history is a bit awkward at the moment. Briefly, any time you can remove an __attrsFailEvaluation = true from nixpkgs without breaking CI, you should do so.
There was a PR (possibly even the last one of mine that was merged) which significantly tightened up CI, so that every top-level attribute had to evaluate when wrapped in builtins.tryEval (there are many Nix failures, like abort and typechecking errors inside C++ code which implements builtins, which result in “untriable failures” – failures which will cause eval to fail even when you wrap them with builtins.tryEval).
I fixed about half of the situations where this happened (the “shallow bugs”) and added __attrsFailEvaluation as a “cheat code” to basically disable that CI check for the other half. The hope is that at some point all of the __attrsFailEvaluation=trues can be eliminated from nixpkgs, and then we can remove the cheat code itself and rg __attrsFailEvaluation nixpkgs will produce zero matches. This will be immediately followed by world peace and flying pigs, of course.
Long answer: if needed (probably not) will have to wait a few days for me to get back to the office.