Skip to content

Commit cfe5bfe

Browse files
committed
Make labels scope to their closest containing choice branch instead of the entire rule.
Previously labels were scoped to the rule. This meant that labels down different choice branches could never be initialized at the same time, but nonetheless needed unique names. In practice, it ended up being common to need to do clumsy things like add number suffixes to label names to distinguish them across choice branches. This is nosiy and error prone: A <- thing0:Thing / "(" thing1:Thing ")" / "{" thing2:Thing "}" With this change, the scope of the name is now bound to the nearest containing choice branch, so the same label name can be reused if they never will never conflict: A <- thing:Thing / "(" thing:Thing ")" / "{" thing:Thing "}"
1 parent e5beeef commit cfe5bfe

8 files changed

Lines changed: 592 additions & 70 deletions

File tree

README.md

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,26 @@ A label is an identifier followed by : followed by an expression.
205205

206206
Labels are used to create new identifiers used by actions and code predicates.
207207

208+
The scope of a label is its branch in the nearest, containing choice expression,
209+
or in the entire rule if there is no choice expression.
210+
211+
For example,
212+
213+
R <- a:A / a:A / a:A / a:A
214+
215+
All `a`s refer to different labels, as they are all scoped to different branches of the choice, `/`.
216+
217+
Similarly, in this expression,
218+
219+
R <- a:A / (a:A / a:A)
220+
221+
all `a`s are different labels.
222+
However,
223+
224+
R <- a:A / a:A a:A
225+
226+
is an error, as `a` is re-defined in the right-hand branch of the choice, `/`.
227+
208228
**Accepts:**
209229
A label accepts if its subexpression accepts.
210230

@@ -349,7 +369,7 @@ The expression must result in a boolean value,
349369
and must be syntactically valid as the condition of an
350370
[if statement](https://golang.org/ref/spec#If_statements).
351371

352-
Label expressions of the containing rule define identifiers accessible in the Go code.
372+
Label expressions in scope of the code predicate define identifiers accessible in the Go code.
353373
The value of the identifier is a `string` of the input consumed by the labeled expression.
354374
If the labeled expression has yet to accept at the time the code predicate is evalutade, the string is empty.
355375

@@ -431,7 +451,7 @@ and the returned value must be one of:
431451
* [a rune literal](https://golang.org/ref/spec#Rune_literals)
432452
* [a string literal](https://golang.org/ref/spec#String_literals)
433453

434-
Label expressions of the containing rule define identifiers accessible in the Go code.
454+
Label expressions in scope of the action define identifiers accessible in the Go code.
435455
The value of the identifier is the value of the labeled expression if it accepted.
436456
If the labeled expression has yet to accept at the time the action is evaluated,
437457
the value is the zero value of the corresponding type.

check.go

Lines changed: 51 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -30,21 +30,8 @@ func Check(grammar *Grammar) error {
3030
for _, r := range rules {
3131
r.checkLeft(ruleMap, p, &errs)
3232
}
33-
34-
var labels []map[string]*LabelExpr
3533
for _, r := range rules {
36-
ls := check(r, ruleMap, &errs)
37-
labels = append(labels, ls)
38-
}
39-
for i, ls := range labels {
40-
rule := rules[i]
41-
for name, expr := range ls {
42-
l := Label{Name: name, Type: expr.Type(), N: expr.N}
43-
rule.Labels = append(rule.Labels, l)
44-
}
45-
sort.Slice(rule.Labels, func(i, j int) bool {
46-
return rule.Labels[i].N < rule.Labels[j].N
47-
})
34+
check(r, ruleMap, &errs)
4835
}
4936
if err := errs.ret(); err != nil {
5037
return err
@@ -238,30 +225,44 @@ func (e *CharClass) checkLeft(rules map[string]*Rule, p path, errs *Errors) {}
238225

239226
func (e *Any) checkLeft(rules map[string]*Rule, p path, errs *Errors) {}
240227

241-
func check(rule *Rule, rules map[string]*Rule, errs *Errors) map[string]*LabelExpr {
242-
labels := make(map[string]*LabelExpr)
243-
rule.Expr.check(rules, labels, true, errs)
244-
return labels
228+
type ctx struct {
229+
rules map[string]*Rule
230+
allLabels *[]*LabelExpr
231+
curLabels map[string]*LabelExpr
245232
}
246233

247-
func (e *Choice) check(rules map[string]*Rule, labels map[string]*LabelExpr, valueUsed bool, errs *Errors) {
248-
for _, sub := range e.Exprs {
249-
sub.check(rules, labels, valueUsed, errs)
234+
func check(rule *Rule, rules map[string]*Rule, errs *Errors) {
235+
ctx := ctx{
236+
rules: rules,
237+
allLabels: &rule.Labels,
238+
curLabels: make(map[string]*LabelExpr),
250239
}
240+
rule.Expr.check(ctx, true, errs)
241+
sort.Slice(rule.Labels, func(i, j int) bool {
242+
return rule.Labels[i].N < rule.Labels[j].N
243+
})
244+
}
251245

252-
t := e.Type()
246+
func (e *Choice) check(ctx ctx, valueUsed bool, errs *Errors) {
247+
for _, sub := range e.Exprs {
248+
subCtx := ctx
249+
subCtx.curLabels = make(map[string]*LabelExpr)
250+
for n, l := range ctx.curLabels {
251+
subCtx.curLabels[n] = l
252+
}
253+
sub.check(subCtx, valueUsed, errs)
254+
}
255+
t := e.Exprs[0].Type()
253256
for _, sub := range e.Exprs {
254-
// Check types, but if either type is "",
255-
// it's from a previous error; don't report again.
256257
if got := sub.Type(); *genActions && valueUsed && got != t && got != "" && t != "" {
257258
errs.add(sub, "type mismatch: got %s, expected %s", got, t)
258259
}
259260
}
260261
}
261262

262-
func (e *Action) check(rules map[string]*Rule, labels map[string]*LabelExpr, valueUsed bool, errs *Errors) {
263-
e.Expr.check(rules, labels, false, errs)
264-
for _, l := range labels {
263+
func (e *Action) check(ctx ctx, valueUsed bool, errs *Errors) {
264+
e.Expr.check(ctx, false, errs)
265+
for _, l := range ctx.curLabels {
265266
e.Labels = append(e.Labels, l)
266267
}
267268
sort.Slice(e.Labels, func(i, j int) bool {
@@ -270,9 +271,9 @@ func (e *Action) check(rules map[string]*Rule, labels map[string]*LabelExpr, val
270271
}
271272

272273
// BUG: figure out what to do about sequence types.
273-
func (e *Sequence) check(rules map[string]*Rule, labels map[string]*LabelExpr, valueUsed bool, errs *Errors) {
274+
func (e *Sequence) check(ctx ctx, valueUsed bool, errs *Errors) {
274275
for _, sub := range e.Exprs {
275-
sub.check(rules, labels, valueUsed, errs)
276+
sub.check(ctx, valueUsed, errs)
276277
}
277278
t := e.Exprs[0].Type()
278279
for _, sub := range e.Exprs {
@@ -282,51 +283,52 @@ func (e *Sequence) check(rules map[string]*Rule, labels map[string]*LabelExpr, v
282283
}
283284
}
284285

285-
func (e *LabelExpr) check(rules map[string]*Rule, labels map[string]*LabelExpr, valueUsed bool, errs *Errors) {
286-
e.Expr.check(rules, labels, true, errs)
287-
if _, ok := labels[e.Label.String()]; ok {
286+
func (e *LabelExpr) check(ctx ctx, valueUsed bool, errs *Errors) {
287+
e.Expr.check(ctx, true, errs)
288+
if _, ok := ctx.curLabels[e.Label.String()]; ok {
288289
errs.add(e.Label, "label %s redefined", e.Label.String())
289290
}
290-
e.N = len(labels)
291-
labels[e.Label.String()] = e
291+
e.N = len(*ctx.allLabels)
292+
*ctx.allLabels = append(*ctx.allLabels, e)
293+
ctx.curLabels[e.Label.String()] = e
292294
}
293295

294-
func (e *PredExpr) check(rules map[string]*Rule, labels map[string]*LabelExpr, valueUsed bool, errs *Errors) {
295-
e.Expr.check(rules, labels, false, errs)
296+
func (e *PredExpr) check(ctx ctx, valueUsed bool, errs *Errors) {
297+
e.Expr.check(ctx, false, errs)
296298
}
297299

298-
func (e *RepExpr) check(rules map[string]*Rule, labels map[string]*LabelExpr, valueUsed bool, errs *Errors) {
299-
e.Expr.check(rules, labels, valueUsed, errs)
300+
func (e *RepExpr) check(ctx ctx, valueUsed bool, errs *Errors) {
301+
e.Expr.check(ctx, valueUsed, errs)
300302
}
301303

302-
func (e *OptExpr) check(rules map[string]*Rule, labels map[string]*LabelExpr, valueUsed bool, errs *Errors) {
303-
e.Expr.check(rules, labels, valueUsed, errs)
304+
func (e *OptExpr) check(ctx ctx, valueUsed bool, errs *Errors) {
305+
e.Expr.check(ctx, valueUsed, errs)
304306
}
305307

306-
func (e *SubExpr) check(rules map[string]*Rule, labels map[string]*LabelExpr, valueUsed bool, errs *Errors) {
307-
e.Expr.check(rules, labels, valueUsed, errs)
308+
func (e *SubExpr) check(ctx ctx, valueUsed bool, errs *Errors) {
309+
e.Expr.check(ctx, valueUsed, errs)
308310
}
309311

310-
func (e *Ident) check(rules map[string]*Rule, _ map[string]*LabelExpr, _ bool, errs *Errors) {
311-
r, ok := rules[e.Name.String()]
312+
func (e *Ident) check(ctx ctx, _ bool, errs *Errors) {
313+
r, ok := ctx.rules[e.Name.String()]
312314
if !ok {
313315
errs.add(e, "rule %s undefined", e.Name.String())
314316
} else {
315317
e.rule = r
316318
}
317319
}
318320

319-
func (e *PredCode) check(_ map[string]*Rule, labels map[string]*LabelExpr, _ bool, _ *Errors) {
320-
for _, l := range labels {
321+
func (e *PredCode) check(ctx ctx, _ bool, _ *Errors) {
322+
for _, l := range ctx.curLabels {
321323
e.Labels = append(e.Labels, l)
322324
}
323325
sort.Slice(e.Labels, func(i, j int) bool {
324326
return e.Labels[i].Label.String() < e.Labels[j].Label.String()
325327
})
326328
}
327329

328-
func (e *Literal) check(map[string]*Rule, map[string]*LabelExpr, bool, *Errors) {}
330+
func (e *Literal) check(ctx, bool, *Errors) {}
329331

330-
func (e *CharClass) check(map[string]*Rule, map[string]*LabelExpr, bool, *Errors) {}
332+
func (e *CharClass) check(ctx, bool, *Errors) {}
331333

332-
func (e *Any) check(map[string]*Rule, map[string]*LabelExpr, bool, *Errors) {}
334+
func (e *Any) check(ctx, bool, *Errors) {}

check_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,16 @@ G <- [fgh]*`,
7878
in: "A <- a:[a] a:[a]",
7979
err: "^test.file:1.12,1.13: label a redefined",
8080
},
81+
{
82+
name: "non-redefined label with same name in different branch",
83+
in: "A <- a:[a] / (a:[a] / a:[a]) / a:[a]",
84+
err: "",
85+
},
86+
{
87+
name: "redefined label in same choice branch",
88+
in: "A <- a:[a] / a:[a] a:[a]",
89+
err: "^test.file:1.20,1.21: label a redefined",
90+
},
8191
{
8292
name: "choice first error",
8393
in: "A <- Undefined / A",

example/calc/calc.go

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@ func main() {
1818
scanner := bufio.NewScanner(os.Stdin)
1919
for scanner.Scan() {
2020
line := scanner.Text()
21-
p := _NewParser(line)
21+
p, err := _NewParser(line)
22+
if err != nil {
23+
fmt.Println(err)
24+
os.Exit(1)
25+
}
2226
if pos, perr := _ExprAccepts(p, 0); pos < 0 {
2327
_, fail := _ExprFail(p, 0, perr)
2428
fmt.Println(peg.SimpleError(line, fail))
@@ -84,15 +88,24 @@ type _key struct {
8488
rule int
8589
}
8690

87-
func _NewParser(text string) *_Parser {
88-
return &_Parser{
91+
type tooBigError struct{}
92+
93+
func (tooBigError) Error() string { return "input is too big" }
94+
95+
func _NewParser(text string) (*_Parser, error) {
96+
n := len(text) + 1
97+
if n < 0 {
98+
return nil, tooBigError{}
99+
}
100+
p := &_Parser{
89101
text: text,
90-
deltaPos: make([][_N]int32, len(text)+1),
91-
deltaErr: make([][_N]int32, len(text)+1),
102+
deltaPos: make([][_N]int32, n),
103+
deltaErr: make([][_N]int32, n),
92104
node: make(map[_key]*peg.Node),
93105
fail: make(map[_key]*peg.Fail),
94106
act: make(map[_key]interface{}),
95107
}
108+
return p, nil
96109
}
97110

98111
func _max(a, b int) int {

example/calc/calc.peggy

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,11 @@ func main() {
1919
scanner := bufio.NewScanner(os.Stdin)
2020
for scanner.Scan() {
2121
line := scanner.Text()
22-
p := _NewParser(line)
22+
p, err := _NewParser(line)
23+
if err != nil {
24+
fmt.Println(err)
25+
os.Exit(1)
26+
}
2327
if pos, perr := _ExprAccepts(p, 0); pos < 0 {
2428
_, fail := _ExprFail(p, 0 ,perr)
2529
fmt.Println(peg.SimpleError(line, fail))

0 commit comments

Comments
 (0)