search

conjure-cp / conjure-oxide

Mozilla Public License 2.0
9 stars 16 forks source link

Remove "either do bubble / bubble or not bubble / not bubble" conditional from div / mod rules #472

Open ozgurakgun opened 3 days ago

ozgurakgun commented 3 days ago

What's the rationale for this?

https://github.com/conjure-cp/conjure-oxide/blob/ff49c1521cfb9b077e8ebe4a585eda6421bb16b8/crates/conjure_core/src/rules/bubble.rs#L84-L88

Asking for no bubbles, I can understand (not for correctness but for performance like the if a.can_be_undefined() || b.can_be_undefined() { check above it). But why are we happy with 0 or 2 bubbles?

niklasdewally commented 3 days ago

Added it to address this comment

https://github.com/conjure-cp/conjure-oxide/pull/450#discussion_r1844330429

I might have misunderstood what you meant here?

ozgurakgun commented 3 days ago

yes, I think that is unrelated. that is about bubbling up and should already be implemented in the generic bubbling up rule.

this rule is about adding bubbles, not bubbling them up.

I think you should be able to just remove that if statement from both div and mod rules - unless I am missing something.