Attachment: Download

Issue Description:

I've stumbled upon this issue intermittently on the latest master branch. It was an issue when working on the code forked of last weeks master. When I wanted to file this bug yesterday, I wasn't able to reproduce on Tuesday's master. I was about to commit a simplified code and remove the workaround for the issue today, but I see the problem again with Wednesday's master.

I'm about to leave for vacation, so I don't have time to reduce it further. All I have is this slightly confusing println debugging patch against master and a test case invocation that demonstrates the issue. I hope you'll be able to reproduce it.

The issue manifests in the code of Swift Benchmark Suite, in the DriverUtils.swift. That code is using failable initializers for numeric types to do the type checking in the checked function. There are two generic functions involved and the type inference has to go through KeyPaths. The parser for --sample-time is a bit more complex, because it has to only accept finite Double values (excluding NaN and ±Infinity), this is done using the optional filter pattern. So the closure body is a chain of failable init and flatmap (the simpler cases with just the failable init, used for other attributes don't have this issue). It is further complicated by the fact that the type of the resulting value (which is stored into the result) is also optional. I'm not sure which part of this rather complex setup is causing the issue, just pointing out to potential leads…

I'm compiling the benchmarks with

ninja -C ${SWIFT_BUILD_DIR} swift-benchmark-macosx-x86_64

Then there's one lit test (prefixed NANVALUE), that checks the error handling of parsing the arguments for Benchmark_O does the type conversion correctly. With the issue present, the test will take very long to fail, because it devolves into running the whole pre-commit test suite instead of failing quickly with the expected error message. Here's a modified invocation that will always fail quickly:

swift-source$ ${SWIFT_BUILD_DIR}/bin/Benchmark_O --sample-time=NaN --list | head -n1
#,Test,[Tags]
swift-source$ ${SWIFT_BUILD_DIR}/bin/Benchmark_O --sample-time=Na --list | head -n1
error: 'Na' is not a valid 'Double' for '--sample-time'

The first invocation with NaN should fail the same way as the second, with the error message.

The issue arises when the parser is specified with untyped inline closure. Adding explicit type information to the closure or converting it to nested function works around the issue. But as the debug messages added to checked (after converting the parser closure to escaping, required by the need to pass it into String) demonstrate, the type information inside the checked function remains the same for the inferred case and explicitly typed case.

First the expected behavior with explicitly typed parser (inline or nested functions both work), including --num-iters : UInt for comparison:

$ Benchmark_O --num-iters=NaN --list | head -n5
error: 'NaN' is not a valid 'UInt' for '--num-iters'
parser type: (Swift.String) throws -> Swift.Optional<Swift.Optional<Swift.UInt>>
failed type conversion: "NaN" :: Swift.Optional<Swift.UInt>.Type
$ Benchmark_O --sample-time=Na --list | head -n5
error: 'Na' is not a valid 'Double' for '--sample-time'
parser type: (Swift.String) throws -> Swift.Optional<Swift.Optional<Swift.Double>>
failed type conversion: "Na" :: Swift.Optional<Swift.Double>.Type
$ Benchmark_O --sample-time=NaN --list | head -n5
error: 'NaN' is not a valid 'Double' for '--sample-time'
parser type: (Swift.String) throws -> Swift.Optional<Swift.Optional<Swift.Double>>
failed type conversion: "NaN" :: Swift.Optional<Swift.Double>.Type

Note: error is printed to stderr, which is why it appears first, even though its later in the code.

The issue with inline type-inferred parser:

$ Benchmark_O --num-iters=NaN --list | head -n5
error: 'NaN' is not a valid 'UInt' for '--num-iters'
parser type: (Swift.String) throws -> Swift.Optional<Swift.Optional<Swift.UInt>>
failed type conversion: "NaN" :: Swift.Optional<Swift.UInt>.Type
$ Benchmark_O --sample-time=Na --list | head -n5
error: 'Na' is not a valid 'Double' for '--sample-time'
parser type: (Swift.String) throws -> Swift.Optional<Swift.Optional<Swift.Double>>
failed type conversion: "Na" :: Swift.Optional<Swift.Double>.Type
$ Benchmark_O --sample-time=NaN --list | head -n5
parser type: (Swift.String) throws -> Swift.Optional<Swift.Optional<Swift.Double>>
succesful type conversion:  "NaN" : nil :: Swift.Optional<Swift.Double>
#,Test,[Tags]
2,AngryPhonebook,[String, api, validation]
3,AnyHashableWithAClass,[abstraction, cpubench, runtime]

It looks to me like there is some funky implicit conversion going on somewhere? Or something is eliminating the flatMap call?