flush stderr when tracing the parser

[hhugo]

Make sure to flush stderr when tracing the parser.
This issue caused me some pain while trying to capture the trace inside an expect test.

see ocsigen/js_of_ocaml#1308 and janestreet/ppx_expect#43

[nojb]

Clearly correct. LGTM. (I checked that there were no other instances of fprintf in this file that needed flushing.)

[dra27]

The code is clearly correct, but I think it's at least worth some clarifying comment, given that the intuitive response is that stderr isn't meant to be buffered¹. This could also be a good point to fold if (trace()) {fprintf(stderr, ...); fflush(stderr)} into a single print_trace function?

Footnotes

1. I think that the truth is that the stderr stream is only allowed to be buffered if the underlying fd is known not to be a console/tty; that said I don't think the MS implementation makes this distinction and I'm not sure if it's actually part of the standard, or just convention. ↩

[hhugo]

The code is clearly correct, but I think it's at least worth some clarifying comment, given that the intuitive response is that stderr isn't meant to be buffered1. This could also be a good point to fold if (trace()) {fprintf(stderr, ...); fflush(stderr)} into a single print_trace function?

I've pushed a commit following your suggestions.

[dra27]

This is good to go, thanks! @gasche - surely this does want a Changes entry?

[gasche]

Indeed you are right, it would be better to have a Changes entry: it fixes a bug and it's written by a less-common contributor. I think that I used the label to make the CI go green, without thinking too much about it. @hhugo please add a Changes entry, probably in the "Bug fixes" section?

[hhugo]

Indeed you are right, it would be better to have a Changes entry: it fixes a bug and it's written by a less-common contributor. I think that I used the label to make the CI go green, without thinking too much about it. @hhugo please add a Changes entry, probably in the "Bug fixes" section?

Done.

Please squash all commits when merging.

[gasche]

The build failure on the OSX test machine comes from intext_par, it is unrelated.

(The rest of this message is independent from this PR, merely about the current CI/testsuite.)

Looking at the raw logs gave me the following information on tests that are surprisingly slow on the OSX test machine. They are probably candidates for tuning to consume less resources (on all systems):

Tests taking longer than 10s:
    tests/basic/tailcalls.ml: 14.83
    tests/lazy/lazy3.ml: 23.27
    tests/lazy/lazy5.ml: 22.69
    tests/lazy/lazy7.ml: 35.78
    tests/lf_skiplist/test_parallel.ml: 12.88
    tests/lib-bytes-utf/test.ml: 23.46
    tests/lib-dynlink-domains/main.ml: 11.96
    tests/lib-dynlink-native/main.ml: 15.75
    tests/lib-systhreads/boundscheck.ml: 25.05
    tests/lib-systhreads/multicore_lifecycle.ml: 14.57
    tests/lib-systhreads/testfork.ml: 14.62
    tests/lib-systhreads/testfork2.ml: 12.21
    tests/lib-systhreads/threadsigmask.ml: 18.46
    tests/lib-threads/signal.ml: 10.32
    tests/lib-threads/torture.ml: 10.35
    tests/match-exception/streams.ml: 13.66
    tests/memory-model/forbidden.ml: 31.00
    tests/memory-model/publish.ml: 13.63
    tests/misc/nucleic.ml: 10.18
    tests/misc/sorts.ml: 17.88
    tests/parallel/fib_threads.ml: 56.76
    tests/parallel/mctest.ml: 16.21
    tests/regression/pr9853/compaction_corner_case.ml: 23.14
    tests/unboxed-primitive-args/test.ml: 13.70
    tests/weak-ephe-final/weaklifetime.ml: 10.16