Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug fixes, improvements, optimization & refactoring before parser generation #288

Merged
merged 13 commits into from Dec 5, 2022
Merged

Bug fixes, improvements, optimization & refactoring before parser generation #288

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5fa25c2
Fix bad float conformation error message + add coverage
Aug 13, 2022
2795a9d
Remove duplicit information from int/float conversion errors
petee-d Dec 4, 2022
ad1557e
Fix possible panic when optional groups are used within a capture
petee-d Dec 4, 2022
5e400b6
Improve mismatch error message for negation & negative lookahead group
petee-d Dec 4, 2022
3de599c
Improve the choice of error reporting after optional branches
petee-d Dec 5, 2022
d03a0d2
Optimize string field accumulation to do fewer allocations
Aug 13, 2022
b36c8c7
Optimize finding case insensitive tokens to be done at parser constru…
Aug 13, 2022
bb4e748
Avoid an allocation in newParseContext
petee-d Dec 4, 2022
bf35800
Include explicit disjunction in the union node
petee-d Dec 4, 2022
9aefebf
Remove unused optional and repetition grammar nodes
Nov 7, 2022
8854a6d
Change the receiver name of lookaheadGroup from n to l
petee-d Dec 4, 2022
af8a3d3
Keep track of the number of usages of a strct node
Aug 13, 2022
a61174d
Allow specifying UnexpectedTokenError expected sequence as a string
Aug 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions nodes.go
View file
Open in desktop
Expand Up @@ -591,15 +591,15 @@ func conform(t reflect.Type, values []reflect.Value) (out []reflect.Value, err e
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
n, err := strconv.ParseInt(v.String(), 0, sizeOfKind(kind))
if err != nil {
return nil, fmt.Errorf("invalid integer %q: %s", v.String(), err)
return nil, err
}
v = reflect.New(t).Elem()
v.SetInt(n)

case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
n, err := strconv.ParseUint(v.String(), 0, sizeOfKind(kind))
if err != nil {
return nil, fmt.Errorf("invalid integer %q: %s", v.String(), err)
return nil, err
}
v = reflect.New(t).Elem()
v.SetUint(n)
Expand All @@ -610,7 +610,7 @@ func conform(t reflect.Type, values []reflect.Value) (out []reflect.Value, err e
case reflect.Float32, reflect.Float64:
n, err := strconv.ParseFloat(v.String(), sizeOfKind(kind))
if err != nil {
return nil, fmt.Errorf("invalid float %q: %s", v.String(), err)
return nil, err
}
v = reflect.New(t).Elem()
v.SetFloat(n)
Expand Down
6 changes: 3 additions & 3 deletions parser_test.go
View file
Open in desktop
Expand Up @@ -1874,11 +1874,11 @@ func TestParseNumbers(t *testing.T) {
}
parser := participle.MustBuild[grammar]()
_, err := parser.ParseString("", `300 0 x`)
require.EqualError(t, err, `grammar.Int: invalid integer "300": strconv.ParseInt: parsing "300": value out of range`)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alecthomas I included the previous commit basically just to make it visible what the error message looked like before. IMHO none of the context being added was actually useful and just duplicated the stuff deeper in the error.

The main reason why I removed it had to do with generated parser performance - my parser avoids allocations for errors as much as possible as most errors are later suppressed, so it just collects some error information while parsing and then only allocates the final error once parsing is done. This made that approach tricky as I would need to depend on fmt just because of these int/uint/float errors and allocate extra stuff that might later be thrown away. Hope it makes sense.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, makes sense 👍

require.EqualError(t, err, `grammar.Int: strconv.ParseInt: parsing "300": value out of range`)
_, err = parser.ParseString("", `-2 -2 x`)
require.EqualError(t, err, `grammar.Uint: invalid integer "-2": strconv.ParseUint: parsing "-2": invalid syntax`)
require.EqualError(t, err, `grammar.Uint: strconv.ParseUint: parsing "-2": invalid syntax`)
_, err = parser.ParseString("", `0 0 nope`)
require.EqualError(t, err, `grammar.Float: invalid float "nope": strconv.ParseFloat: parsing "nope": invalid syntax`)
require.EqualError(t, err, `grammar.Float: strconv.ParseFloat: parsing "nope": invalid syntax`)
result, err := parser.ParseString("", `-30 3000 Inf`)
require.NoError(t, err)
require.Equal(t, grammar{Int: -30, Uint: 3000, Float: math.Inf(1)}, *result)
Expand Down