Skip to content

Commit 6e12d5f

Browse files
authored
Fix a type instability in a few error paths that result in the parsedigits (#188)
return type being unstable when an apply function is passed. Made a new helper with a detailed comment explaining the type instability + rules to follow to hopefully avoid these issues in the future.
1 parent 3380022 commit 6e12d5f

File tree

1 file changed

+19
-12
lines changed

1 file changed

+19
-12
lines changed

src/floats.jl

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,13 @@ end
226226

227227
rettype(::Type{T}) where {T} = T === Number ? Nothing : T
228228

229+
# when a function is passed to apply to the parsed value, we have to be really careful that the `x`
230+
# returned from parsedigits is `Nothing`, otherwise we end up with an overall return type like
231+
# `Tuple{Union{Nothing, T}, ReturnCode, Int}`, which can cause dynamic dispatches/inlining issues
232+
# for callers. The rule to follow is that before any call to `@goto done`, we need to ensure `x`
233+
# has been "handled" properly: either applied or in error cases, set to `nothing`
234+
getx(x, f) = f === nothing ? x : nothing
235+
229236
# if we need to _widen the type due to `digits` overflow, we want a non-inlined version so base case compilation doesn't get out of control
230237
@noinline _parsedigits(conf::AbstractConf{T}, source, pos, len, b, code, options, digits::IntType, neg::Bool, startpos, overflow_invalid::Bool, ndigits::Int, f::F) where {T, IntType, F} =
231238
parsedigits(conf, source, pos, len, b, code, options, digits, neg, startpos, overflow_invalid, ndigits, f)::Tuple{rettype(T), ReturnCode, Int}
@@ -250,7 +257,7 @@ rettype(::Type{T}) where {T} = T === Number ? Nothing : T
250257
fastseek!(source, startpos - 1)
251258
pos = startpos
252259
code |= INVALID
253-
x = f === nothing ? x : nothing
260+
x = getx(x, f)
254261
@goto done
255262
end
256263
digits = _muladd(ten(IntType), digits, b)
@@ -271,13 +278,13 @@ rettype(::Type{T}) where {T} = T === Number ? Nothing : T
271278
@goto done
272279
end
273280
elseif has_groupmark && b == groupmark0
274-
prev_b0 == groupmark0 && (code |= INVALID; @goto done) # two groupmarks in a row
281+
prev_b0 == groupmark0 && (code |= INVALID; x = getx(x, f); @goto done) # two groupmarks in a row
275282
pos += 1
276283
Parsers.incr!(source)
277-
Parsers.eof(source, pos, len) && (code |= INVALID | EOF; @goto done) # groupmark at end of input
284+
Parsers.eof(source, pos, len) && (code |= INVALID | EOF; x = getx(x, f); @goto done) # groupmark at end of input
278285
else
279286
# if `b` isn't a digit or a groupmark, time to break out of digit parsing while loop
280-
((has_groupmark && prev_b0 == groupmark0) || !anydigits) && (code |= INVALID; @goto done) # ended with groupmark
287+
((has_groupmark && prev_b0 == groupmark0) || !anydigits) && (code |= INVALID; x = getx(x, f); @goto done) # ended with groupmark
281288
break
282289
end
283290
prev_b0 = b
@@ -306,7 +313,7 @@ rettype(::Type{T}) where {T} = T === Number ? Nothing : T
306313
# otherwise ok, like "1.a" (only "1." is parsed)
307314
if !anydigits
308315
code |= INVALID
309-
x = f === nothing ? x : nothing
316+
x = getx(x, f)
310317
@goto done
311318
else
312319
if T === Number
@@ -356,7 +363,7 @@ end
356363
# input is simple non-scientific-notation floating number, like "1.1"
357364
if overflow_invalid && -signed(frac) > 308
358365
code |= INVALID
359-
x = f === nothing ? x : nothing
366+
x = getx(x, f)
360367
else
361368
x, code = scale(conf, FT, digits, -signed(frac), neg, code, ndigits, f, options)
362369
code |= OK | EOF
@@ -381,7 +388,7 @@ end
381388
if eof(source, pos, len)
382389
# it's an error to have a "dangling" 'e', so input was something like "1.1e"
383390
code |= INVALID | EOF
384-
x = f === nothing ? x : nothing
391+
x = getx(x, f)
385392
@goto done
386393
end
387394
b = peekbyte(source, pos)
@@ -393,7 +400,7 @@ end
393400
if eof(source, pos, len)
394401
# it's an error to have a "dangling" '-' or '+', so input was something like "1.1e-"
395402
code |= INVALID | EOF
396-
x = f === nothing ? x : nothing
403+
x = getx(x, f)
397404
@goto done
398405
end
399406
b = peekbyte(source, pos)
@@ -402,7 +409,7 @@ end
402409
if b > 0x09
403410
# invalid to have a "dangling" 'e'
404411
code |= INVALID
405-
x = f === nothing ? x : nothing
412+
x = getx(x, f)
406413
@goto done
407414
end
408415

@@ -414,7 +421,7 @@ end
414421
if parsedanyfrac
415422
if overflow_invalid && -signed(frac) > 308
416423
code |= INVALID
417-
x = f === nothing ? x : nothing
424+
x = getx(x, f)
418425
@goto done
419426
else
420427
x, code = scale(conf, FT, digits, -signed(frac), neg, code, ndigits, f, options)
@@ -446,7 +453,7 @@ end
446453
ee = ifelse(negexp, -signed(exp), signed(exp)) - signed(frac)
447454
if overflow_invalid && ee > 308
448455
code |= INVALID
449-
x = f === nothing ? x : nothing
456+
x = getx(x, f)
450457
else
451458
x, code = scale(conf, FT, digits, ee, neg, code, ndigits, f, options)
452459
code |= OK | EOF
@@ -459,7 +466,7 @@ end
459466
ee = ifelse(negexp, -signed(exp), signed(exp)) - signed(frac)
460467
if overflow_invalid && ee > 308
461468
code |= INVALID
462-
x = f === nothing ? x : nothing
469+
x = getx(x, f)
463470
else
464471
x, code = scale(conf, FT, digits, ifelse(negexp, -signed(exp), signed(exp)) - signed(frac), neg, code, ndigits, f, options)
465472
code |= OK

0 commit comments

Comments
 (0)