|
| 1 | + |
| 2 | + |
| 3 | +## For rule.cfg |
| 4 | + |
| 5 | +optional vs rule-union vs cfg-union? |
| 6 | + |
| 7 | +* Optional: feels verbose. Requires extra get() calls. |
| 8 | +* Optional: seems harder to detect value |
| 9 | +* Rule-union: which API feels verbose. |
| 10 | +* Cfg-Union: seems nicest? More underlying impl work though. |
| 11 | + |
| 12 | +``` |
| 13 | +# optional |
| 14 | +# Rule.cfg is type Optional[TransitionBuilder | ConfigNone | ConfigTarget] |
| 15 | +
|
| 16 | +r = RuleBuilder() |
| 17 | +cfg = r.cfg.get() |
| 18 | +if <cfg is TransitionBuilder>: |
| 19 | + cfg.inputs.append(...) |
| 20 | +elif <cfg is config.none>: |
| 21 | + ... |
| 22 | +elif <cfg is config.target>: |
| 23 | + ... |
| 24 | +else: error() |
| 25 | +
|
| 26 | +# rule union |
| 27 | +# Rule has {get,set}{cfg,cfg_none,cfg_target} functions |
| 28 | +# which() tells which is set. |
| 29 | +# Setting one clears the others |
| 30 | +
|
| 31 | +r = RuleBuilder() |
| 32 | +which = r.cfg_which() |
| 33 | +if which == "cfg": |
| 34 | + r.cfg().inputs.append(...) |
| 35 | +elif which == "cfg_none": |
| 36 | + ... |
| 37 | +elif which == "cfg_target": |
| 38 | + ... |
| 39 | +else: error |
| 40 | +
|
| 41 | +# cfg union (1) |
| 42 | +# Rule.cfg is type RuleCfgBuilder |
| 43 | +# RuleConfigBuilder has {get,set}{implementation,none,target} |
| 44 | +# Setting one clears the others |
| 45 | +
|
| 46 | +r = RuleBuilder() |
| 47 | +
|
| 48 | +if r.cfg.implementation(): |
| 49 | + r.cfg.inputs.append(...) |
| 50 | +elif r.cfg.none(): |
| 51 | + ... |
| 52 | +elif r.cfg.target(): |
| 53 | + ... |
| 54 | +else: |
| 55 | + error |
| 56 | +
|
| 57 | +# cfg-union (2) |
| 58 | +# Make implementation attribute polymorphic |
| 59 | +impl = r.cfg.implementation() |
| 60 | +if impl == "none": |
| 61 | + ... |
| 62 | +elif impl == "target": |
| 63 | + ... |
| 64 | +else: # function |
| 65 | + r.cfg.inputs.append(...) |
| 66 | +
|
| 67 | +# cfg-union (3) |
| 68 | +# impl attr is an Optional |
| 69 | +impl = r.cfg.implementation.get() |
| 70 | +... r.cfg.implementation.set(...) ... |
| 71 | +``` |
| 72 | + |
| 73 | +## Copies copies everywhere |
| 74 | + |
| 75 | +To have a nicer API, the builders should provide mutable lists/dicts/etc. |
| 76 | + |
| 77 | +But, when they accept a user input, they can't tell if the value is mutable or |
| 78 | +not. So they have to make copies. Most of the time, the values probably _will_ |
| 79 | +be mutable (why use a builder if its not mutable?). But its an easy mistake to |
| 80 | +overlook that a list is referring to e.g. some global instead of a local var. |
| 81 | + |
| 82 | +So, we could defensively copy, or just document that a mutable input is |
| 83 | +expected, and behavior is undefined otherwise. |
| 84 | + |
| 85 | +Alternatively, add a function to py_internal to detect immutability, and it'll |
| 86 | +eventually be available in some bazel release. |
| 87 | + |
| 88 | +## Collections of of complex objects |
| 89 | + |
| 90 | +Should these be exposed as the raw collection, or a wrapper? e.g. |
0 commit comments