Skip to content

Conversation

@jjtolton
Copy link

@jjtolton jjtolton commented Oct 22, 2025

Closes: #302 (comment)

Context of initial comments

⚠️ Work In Progress / Proof of Concept

This PR is opened for discussion purposes. The implementation demonstrates the feasibility of improving syntax error reporting but should be reviewed by someone more experienced with Scryer Prolog internals before merging.

Problem
Currently, when syntax errors occur during file loading in Scryer Prolog, the error messages are not helpful for debugging:

?- main.
%@ error(syntax_error(incomplete_reduction),read_term/3:10).
This error only shows read_term/3:10 - the predicate where the error was caught, not the actual source file that contains the syntax error. This makes it extremely difficult to debug, especially when loading multiple files.

Solution
This PR modifies the error reporting system to include the actual source file name and line number:

error(syntax_error(incomplete_reduction),'test_file.pl':8).
Now developers can immediately identify:

Which file has the syntax error
Which line in that file is problematic
Changes
Modified Files
src/machine/machine_errors.rs

Added file_name: Option field to MachineError struct
Added set_file_name() method for setting the file name
Updated error_form() to include file name in error context when available
Updated all MachineError constructors to initialize file_name: None
src/machine/machine_state.rs

Modified read_term() error handling to extract file name from stream
Calls err.set_file_name(stream.file_name()) to propagate file information
Testing
Created a test file with a deliberate syntax error and verified the output:

Before:

error(syntax_error(incomplete_reduction),read_term/3:10).
After:

error(syntax_error(incomplete_reduction),'test_syntax_error.pl':8).
All existing tests pass with no regressions.

Impact
This is a significant quality-of-life improvement for Scryer Prolog developers. The change:

✅ Maintains backward compatibility (error structure remains the same)
✅ Only adds information when available (doesn't break non-file streams)
✅ Follows existing error reporting patterns
✅ Has minimal performance impact
Discussion Points
This is a proof-of-concept implementation. Questions for maintainers:

Is this the right approach for propagating file information through errors?
Should we handle other error types similarly?
Are there edge cases or scenarios not covered?

@jjtolton jjtolton changed the title Improve syntax error reporting by including source file names [WIP/POC] Improve syntax error reporting by including source file names Oct 22, 2025
@UWN
Copy link

UWN commented Oct 23, 2025

Do no forget #302

All can be implemented with call_with_error_context/2.

@UWN
Copy link

UWN commented Oct 23, 2025

This PR modifies the error reporting system to include the actual source file name and line number:

error(syntax_error(incomplete_reduction),'test_file.pl':8).

This is not a solution. Leave the hacked implementation defined part as is, and add additional information with call_with_error_context/2. That is exactly the reason why it is there.

When syntax errors occur during file loading, the error messages now
include the source file name through call_with_error_context/2 as
recommended in issue mthom#302.

Before:
  error(syntax_error(incomplete_reduction),read_term/3:8).

After:
  error(syntax_error(incomplete_reduction),[file-'test_file.pl'|read_term/3:8]).

This approach uses the existing call_with_error_context/2 mechanism
instead of modifying the core error struct, following the established
pattern for enhancing error context in Scryer Prolog.

Changes:
- Modified stream_next_term/4 in src/loader.pl to wrap read_term/3
  calls with call_with_error_context/2, providing file context when
  available

Resolves: mthom#302
Related: mthom#3133

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@jjtolton jjtolton force-pushed the improve-syntax-error-reporting branch from f9ec456 to ace10bb Compare October 23, 2025 14:56
@jjtolton
Copy link
Author

This PR modifies the error reporting system to include the actual source file name and line number:

error(syntax_error(incomplete_reduction),'test_file.pl':8).

This is not a solution. Leave the hacked implementation defined part as is, and add additional information with call_with_error_context/2. That is exactly the reason why it is there.

Have a question here if that is what the result is supposed to look like

@triska
Copy link
Contributor

triska commented Oct 23, 2025

It's interesting how much shorter and self-contained the change on the Prolog level is.

@jjtolton jjtolton changed the title [WIP/POC] Improve syntax error reporting by including source file names Improve syntax error reporting by including source file names Oct 24, 2025
jjtolton added a commit to jjtolton/scryer-prolog that referenced this pull request Oct 25, 2025
When syntax errors occur during file loading, the error messages now
include the source file name through call_with_error_context/2 as
recommended in issue mthom#302.
Before:
  error(syntax_error(incomplete_reduction),read_term/3:8).
After:
  error(syntax_error(incomplete_reduction),[file-'test_file.pl'|read_term/3:8]).
This approach uses the existing call_with_error_context/2 mechanism
instead of modifying the core error struct, following the established
pattern for enhancing error context in Scryer Prolog.
Changes:
- Modified stream_next_term/4 in src/loader.pl to wrap read_term/3
  calls with call_with_error_context/2, providing file context when
  available
Resolves: mthom#302
Related: mthom#3133
jjtolton added a commit to jjtolton/scryer-prolog that referenced this pull request Oct 25, 2025
When syntax errors occur during file loading, the error messages now
include the source file name through call_with_error_context/2 as
recommended in issue mthom#302.

Before:
  error(syntax_error(incomplete_reduction),read_term/3:8).

After:
  error(syntax_error(incomplete_reduction),[file-'test_file.pl'|read_term/3:8]).

This approach uses the existing call_with_error_context/2 mechanism
instead of modifying the core error struct, following the established
pattern for enhancing error context in Scryer Prolog.

Changes:
- Modified stream_next_term/4 in src/loader.pl to wrap read_term/3
  calls with call_with_error_context/2, providing file context when
  available

Resolves: mthom#302
Related: mthom#3133
jjtolton added a commit to jjtolton/scryer-prolog that referenced this pull request Nov 6, 2025
Updated expected output to match the new error format that includes
file context: [file-'syntax_error.pl'|read_term/3:6] instead of just
read_term/3:6.

This aligns with PR mthom#3133 which improved syntax error reporting to
include source file names.

Co-Authored-By: J.J.'s Robot <jjtolton@gmail.com>
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.

Syntax errors need two positions

3 participants