-
Notifications
You must be signed in to change notification settings - Fork 21
Description
Hey, human here!
The below bug was diagnosed by the Augment Intent app using Opus 4.6, when I had trouble parsing the NIST simplified test files with ruststep.
I skimmed the problem and the solution, and it seemed plausibly correct to me, so it was worth letting the robot go file this issue, because it's clearly not parsing these files that are available in the wild as "generated by real CAD programs" examples.
There's also a problem with multiple coincident vertices. I worked around this by welding such vertices to the first instance on loading, and building a table of duplicate-removed vertices with their forwarding information to be able to backtrack the changes. We'll see how that works out in practice.
I highly recommend using these models as test cases for real CAD files:
https://www.nist.gov/ctl/smart-connected-systems-division/smart-connected-manufacturing-systems-group/mbe-pmi-0
Bug Description
The ruststep parser (version 0.4.0) fails to parse STEP files containing empty parameter lists (), which are valid according to the ISO 10303-21 grammar specification.
Reproduction
Any STEP file containing an empty parameter list will fail to parse. For example:
ANNOTATION_PLANE('name',(#123),#456,())
This results in a TokenizeFailed error.
Expected Behavior
Empty parameter lists () should parse successfully, as they are valid according to the ISO 10303-21 grammar specification which defines parameter lists as optional.
Actual Behavior
The parser fails with a tokenization error when encountering empty parameter lists.
Root Cause
The bug is located in src/parser/exchange/parameter.rs in the list() function (around line 47).
The grammar comment in the code correctly states that parameter lists are optional:
/// \[ [parameter] { , [parameter] } \]
However, the implementation uses comma_separated(parameter), which requires at least one element:
pub fn list(input: &str) -> ParseResult<ParameterList> {
delimited(tag("("), comma_separated(parameter), tag(")"))(input)
}The comma_separated combinator does not allow for zero elements, causing the parser to fail on empty lists.
Suggested Fix
The list() function should use opt(comma_separated(parameter)) or a similar combinator that allows zero elements, to properly handle empty parameter lists according to the grammar specification.
Reference
- ISO 10303-21 (STEP file format specification)
- Affected file:
src/parser/exchange/parameter.rs