search

MichaelHatherly / CommonMark.jl

A CommonMark-compliant Markdown parser for Julia.
Other
87 stars 11 forks source link

Fix inconsistent interpolation evaluation due to Julia bug #50

Closed mortenpi closed 2 years ago

mortenpi commented 2 years ago

I noticed that the tests that parse this string

value = 1
ast = cm"$(value) $(value + 1) $(value += 1) $(value += 1)"

intermittently fail. It turns out that assignments in argument lists (which cm"" depends on, when it constructs the vector of values) are problematic: https://github.com/JuliaLang/julia/issues/46251

This changes the cm"" macro such that the generated code evaluates the values one-by-one and pushes them into a vector that later gets passed to _interp!, rather than constructing the vector in one go with a [ ... ] expression. This should make sure that values are evaluated in a deterministic order.

I split the commits to see if the MWE test also fails on CI systematically, but I think the PR should be squash-merged.

mortenpi commented 2 years ago

I just discovered that the inconsistent evaluation is probably consistent with normal string interpolation:

julia> value = 1; "$(value), $(value + 1), $(value += 1), $(value += 1)"
"3, 2, 2, 3"

julia> let value = 1; "$(value), $(value + 1), $(value += 1), $(value += 1)"; end
"1, 2, 2, 3"

So I am not sure if CommonMark should fix this for cm"", or stay consistent with normal string interpolation.

MichaelHatherly commented 2 years ago

Ooh, very good point... I reckon best to keep the "fixed" behaviour you've added here rather than matching a bug upstream, which, hopefully, will get fixed at some point.