Skip to content

fix(parser): preserve quoted-key text in redefinition error messages#302

Merged
marzer merged 5 commits into
marzer:masterfrom
SAY-5:fix-issue-300-quoted-key-error
May 12, 2026
Merged

fix(parser): preserve quoted-key text in redefinition error messages#302
marzer merged 5 commits into
marzer:masterfrom
SAY-5:fix-issue-300-quoted-key-error

Conversation

@SAY-5
Copy link
Copy Markdown
Contributor

@SAY-5 SAY-5 commented May 11, 2026

What does this change do?

Fixes the corrupted quoted-key string that appears in redefinition error messages. A key written as ["foo"] previously produced an error of the form cannot redefine existing table '"fofoo"'. With this change the diagnostic shows "foo", as written in the source.

Is it related to an exisiting bug report or feature request?

Fixes #300.

Pre-merge checklist

  • I've read CONTRIBUTING.md
  • I've rebased my changes against the current HEAD of origin/master (if necessary)
  • I've added new test cases to verify my change
  • I've regenerated toml.hpp (how-to)
  • I've updated any affected documentation
  • I've rebuilt and run the tests with at least one of:
    • Clang 8 or higher
    • GCC 8 or higher
    • MSVC 19.20 (Visual Studio 2019) or higher
  • I've added my name to the list of contributors in README.md

The parse_string lookahead advanced two code points to detect multi-line
strings, but only the input stream was rewound via go_back(2u) when the
string turned out to be single-line. The diagnostic recording_buffer kept
those code points, so parse_basic_string then re-recorded them and
duplicated the first two characters of every quoted key, producing
messages like 'cannot redefine existing table "fofoo"' for a key named
"foo".

Snapshot the recording_buffer length before the lookahead and restore it
alongside the input rewind so the buffer mirrors the stream state.

Fixes marzer#300.
Copy link
Copy Markdown
Owner

@marzer marzer left a comment

Choose a reason for hiding this comment

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

This fix looks very LLM-coded. The way the comments are written, and the fact that my PR template was annihilated wholesale, yet it's requirements were still referenced.

... Am I right? If so, please remove the LLM junk comments, restore the PR description, and complete the template by hand.

@SAY-5
Copy link
Copy Markdown
Contributor Author

SAY-5 commented May 11, 2026

Acknowledged, closing. Sorry for the noise.

@SAY-5 SAY-5 closed this May 11, 2026
@marzer
Copy link
Copy Markdown
Owner

marzer commented May 11, 2026

@SAY-5 to be clear, I don't have a problem with you using an LLM to arrive at a fix or whatever, just don't blindly go "cursor go brrrrr" in submitting it.

If you want to contribute your fix while also observing project norms I'll happily accept it.

@SAY-5 SAY-5 reopened this May 12, 2026
@SAY-5
Copy link
Copy Markdown
Contributor Author

SAY-5 commented May 12, 2026

@marzer apologies for the noise earlier. Reopened with three things addressed:

  1. Trimmed the two LLM-style narrative comments in parser.inl down to a single line near the snapshot, since the variable name and the surrounding go_back(2u) already explain the rollback.
  2. Trimmed the matching comments in the regenerated toml.hpp.
  3. Restored the PR description so it follows the template verbatim and ticked the boxes that actually apply.

The fix itself is unchanged. The PR currently lacks (a) updating README.md contributors and (b) verifying on Clang/MSVC. Happy to add the README entry on request, and to add those compiler check-marks once I can verify on those toolchains.

@marzer
Copy link
Copy Markdown
Owner

marzer commented May 12, 2026

Thanks, I appreciate it.

  • I'm ok with CI hitting the other toolchains, so long as you've hit it on at least one.
  • Adding to the contributor list is optional, of course :). I added that because I wanted to ensure people remembered to record their work here, and it would survive git. Totally OK if you don't want to do that, no dramas.

I'll look at the rest now.

@marzer
Copy link
Copy Markdown
Owner

marzer commented May 12, 2026

Looks good, happy to merge pending any last-second editorial changes.

@marzer marzer merged commit f22f035 into marzer:master May 12, 2026
15 checks passed
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.

wrong string in error message

2 participants