refactor: NodeType constructors, adding new_auto

[acl-cqc]

• Rename NodeType::open_extensions to NodeType::new_open
• Rename NodeType::pure to NodeType::new_pure
• Add NodeType::new_auto, which uses Pure for module-ops and Open for others
• Remove special-case in infer.rs solving some module-ops to empty set
• Switch builder/HugrMut methods from new_open to new_auto

[codecov]

Codecov Report

Merging #635 (740c394) into main (2e504d0) will increase coverage by 0.02%.
The diff coverage is 98.27%.

@@            Coverage Diff             @@
##             main     #635      +/-   ##
==========================================
+ Coverage   78.57%   78.59%   +0.02%     
==========================================
  Files          65       65              
  Lines       11341    11348       +7     
  Branches    11341    11348       +7     
==========================================
+ Hits         8911     8919       +8     
+ Misses       1793     1792       -1     
  Partials      637      637              

Files	Coverage Δ
src/builder.rs	86.84% <100.00%> (ø)
src/builder/build_traits.rs	80.48% <100.00%> (ø)
src/builder/cfg.rs	85.55% <100.00%> (ø)
src/builder/dataflow.rs	84.31% <100.00%> (ø)
src/builder/module.rs	68.27% <100.00%> (ø)
src/builder/tail_loop.rs	76.22% <100.00%> (ø)
src/extension/infer.rs	86.45% <100.00%> (+0.05%)	⬆️
src/hugr.rs	90.45% <100.00%> (+0.29%)	⬆️
src/hugr/hugrmut.rs	68.70% <100.00%> (ø)
src/hugr/serialize.rs	92.13% <100.00%> (-0.06%)	⬇️
... and 4 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[croyzor]

I think while we're doing this, we should probably go for new_open and new_closed or just open and closed (I prefer the latter). The name for the new_default could be clearer about what it does. If it were actually the default option, it should just be called new and used as the de facto default, but I think it's a bit to unintuative for that

[acl-cqc]

I think while we're doing this, we should probably go for new_open and new_closed or just open and closed (I prefer the latter).

Happy to change pure to closed. (I never really liked "pure", but I admit changing it to "closed" does make me think maybe "pure" isn't so bad! Closed is opposite to open so should be any fixed ExtensionSet ??)

I'm pretty clear these should be new_ methods, the alternative would be to make them methods on Op (like FunctionType::pure, which I've renamed to FunctionType::closed). The unfortunate thing there is that that's a trait, so we could define a Trait-default fn open(self) -> NodeType but we'd have to be saying, Trait implementors should never override this...

The name for the new_default could be clearer about what it does. If it were actually the default option, it should just be called new and used as the de facto default, but I think it's a bit to unintuitive for that

I take new as being the maximally-explicit version that allows anything; the alternative would be to rename new to new_with_extensions or something (except that it can also accept None = extensions unspecified!). Then we could rename new_default to new but as you say I think that'd be rather surprising.

So instead I've renamed new_default to new_auto, how's that?

[croyzor]

You make a good point, closed could apply to any fixed extension set, so I'm happy to revert to pure.
I feel the symmetry of open/closed constructors is somehow less important when they're prefixed with new, so if we're sticking with that then it's all good.

So instead I've renamed new_default to new_auto, how's that?

👍

[acl-cqc]

You make a good point, closed could apply to any fixed extension set, so I'm happy to revert to pure. I feel the symmetry of open/closed constructors is somehow less important when they're prefixed with new, so if we're sticking with that then it's all good.

So instead I've renamed new_default to new_auto, how's that?

👍

Ok, so new_pure, new_open, and new_auto it is...