Serialize writes a StringSet to std_formatter instead of the supplied formatter

Open Beginner friendly
#40 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
1/5
Estimated time
Under an hour
Newbie friendliness
92/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
ocaml
Domain
tooling

Research direction

Start in lib/serialize.ml:26-28 and compare this printer with the corrected printers in lib/value.ml and lib/solverAst.ml from #35. Run the repro in test/test_cases/sets.gbl and verify that formatting a term containing a StringSet keeps the set inside the supplied formatter output without stray terminal output.

Written by the indexing model from the issue text.

Description

lib/serialize.ml:26-28:

| Leaf (StringSet s) ->
  Format.fprintf Format.std_formatter "{%a}"
    (Lib.pp_print_list Format.pp_print_string ", ") (Utils.StringSet.to_list s)

It ignores ppf and writes to stdout, so Format.asprintf on a term containing a set silently drops the set from the result and prints it to the terminal instead.

Repro

test/test_cases/sets.gbl:

$ goblin --file sets.gbl --seed 42
{, a}(S@{0}[0] (Names@{0}[0] (set.union (set.singleton "") (set.singleton "a"))) (Chosen@{0}[0] ""))

The leading {, a} is the stray write; it appears before the term rather than inside it.

Notes

  • Pre-existing: identical on 9c1f1e9.
  • The same bug was fixed in lib/value.ml and lib/solverAst.ml in #35 (two printers that wrote to std_formatter regardless of the supplied ppf); this is the one remaining instance of that pattern.
Dominant language
OCaml
Stars
0
Forks
1
Avg merge
1h 57m
Merged PRs (30d)
2

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from lorchrob/Goblin

All issues in lorchrob/Goblin

Similar issues

More DevTools issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.