Skip to content

Commit 24616e2

Browse files
committed
wip: moar builders
1 parent edfb4b3 commit 24616e2

17 files changed

+1206
-420
lines changed

.bazelversion

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
8.x
1+
8.0.1

api_notes.md

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
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.

0 commit comments

Comments
 (0)