Skip to content

Improve interface parser diagnostics - #190

Open
dellaert wants to merge 1 commit into
masterfrom
codex/parser-diagnostics
Open

Improve interface parser diagnostics#190
dellaert wants to merge 1 commit into
masterfrom
codex/parser-diagnostics

Conversation

@dellaert

Copy link
Copy Markdown
Member

Summary

  • add a shared InterfaceParseError with source filename, line, column, source excerpt, caret, parser context, and optional hints
  • retain the deepest credible pyparsing failure that would otherwise be swallowed by ZeroOrMore or Optional backtracking
  • pass .i source names through the existing wrapper entrypoints and print clean command-line diagnostics without Python tracebacks
  • replace constructor/operator parse-time assertions with located semantic errors

Root cause

The shared .i grammar uses repeated and optional expressions. When a declaration partially matched and then failed, pyparsing backtracked out of the declaration. The remaining outer rule usually reported Expected string_end or Expected '}' at the start of the enclosing class or namespace, discarding the useful nested failure.

Tested examples

Missing argument name

namespace gtsam {
class Foo {
  void project(const Pose3&);
};
}
example.i:3:28: parse error in argument: expected argument name
 3 |   void project(const Pose3&);
   |                            ^
hint: wrapper interface arguments require names, for example 'const Pose3& pose'

Missing method semicolon

class Foo { void bar() };
example.i:1:24: parse error in method declaration: expected ';' after method declaration
 1 | class Foo { void bar() };
   |                        ^

Malformed template argument list

class Foo { void bar(std::vector<double x); };
example.i:1:41: parse error in templated type: expected ',' or '>' after template argument
 1 | class Foo { void bar(std::vector<double x); };
   |                                         ^

Additional regression cases

  • trailing enum comma, with a hint to remove it
  • unsupported public:, private:, or protected: labels in wrapper class declarations
  • malformed arguments nested inside namespaces and classes
  • unclosed namespace braces
  • constructor names that do not match their class
  • operator overloads with too many arguments
  • diagnostic state isolation across consecutive parses
  • operator validation under optimized Python (python -O)
  • clean CLI stderr with exit status 1 and no traceback

Validation

  • conda run -n py312 python -m pytest tests — 110 passed
  • git diff --check
  • tested with pyparsing 3.2.5, matching the project lockfile

@dellaert
dellaert marked this pull request as ready for review July 27, 2026 11:56
@dellaert
dellaert requested review from ProfFan and Copilot July 27, 2026 11:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves wrapper interface (.i) parser diagnostics by introducing a structured InterfaceParseError, capturing the most relevant nested pyparsing failures during backtracking, and surfacing clean, source-located errors (including semantic validations) through the CLI wrappers.

Changes:

  • Add InterfaceParseError plus a diagnostics collector to preserve the deepest credible parser failure and render source excerpts/carets.
  • Thread source_name through parsing entrypoints and wrapper scripts, and print parse errors without Python tracebacks (exit status 1).
  • Replace constructor/operator parse-time assertions with located semantic errors and add comprehensive diagnostic regression tests.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_parser_diagnostics.py Adds regression tests validating structured, source-aware syntax/semantic diagnostics and clean CLI stderr.
scripts/pybind_wrap.py Catches InterfaceParseError, prints clean diagnostic to stderr, and exits with status 1 (no traceback).
scripts/matlab_wrap.py Catches InterfaceParseError and exits with status 1 after printing the diagnostic.
gtwrap/pybind_wrapper.py Plumbs source_name into parsing so diagnostics report the correct filename.
gtwrap/matlab_wrapper/wrapper.py Plumbs a composed source_name into parsing for single/multi-file wraps.
gtwrap/interface_parser/module.py Wraps pyparsing failures into InterfaceParseError using the diagnostic context.
gtwrap/interface_parser/function.py Improves argument-name failure messaging by naming the IDENT subexpression.
gtwrap/interface_parser/enum.py Improves enumerator-name failure messaging by naming the IDENT subexpression.
gtwrap/interface_parser/diagnostics.py New diagnostic machinery (failure tracking, best-failure selection, source rendering, semantic errors).
gtwrap/interface_parser/classes.py Converts constructor/operator semantic checks into located semantic_error exceptions.
gtwrap/interface_parser/init.py Exports InterfaceParseError and registers tracked rules for better backtracked-failure selection.
Comments suppressed due to low confidence (2)

gtwrap/interface_parser/diagnostics.py:274

  • ParserElement's scanning API is typically exposed as scanString(...) (camelCase). Using .scan_string(...) here risks an AttributeError at runtime, and should be aligned with the canonical pyparsing method name (and the updated cppStyleComment/quotedString identifiers).
        (cpp_style_comment | quoted_string).scan_string(source))

gtwrap/interface_parser/diagnostics.py:196

  • ParserElement is used with camelCase APIs throughout this repo (setParseAction, enablePackrat, etc.). Using set_fail_action here may rely on a non-standard alias and can fail at import/runtime if it is unavailable. Prefer the canonical pyparsing API name setFailAction for compatibility.
    rule.set_fail_action(record_failure)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread gtwrap/interface_parser/diagnostics.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants