I’m not clear on what the reflection tests are attempting to actually verify. Just that we don’t change the internal layout of these types accidentally? We’re churning up the layouts of a lot of the collections to get things all set up for ABI stability, which means mechanically updating these tests to expect “whatever output we now happen to output”.
In Dave’s initial eager bridging stuff he left a comment indicating that these are incorrectly relying on implementation details. Without any context, I’m inclined to agree. The fact that somewhere deep in the guts of String there lives an enum doesn’t seem important to verify. (how big it is, and how many extra inhabitants it has, does seem worth verifying longterm).
I’m not clear on what the reflection tests are attempting to actually verify. Just that we don’t change the internal layout of these types accidentally? We’re churning up the layouts of a lot of the collections to get things all set up for ABI stability, which means mechanically updating these tests to expect “whatever output we now happen to output”.
In Dave’s initial eager bridging stuff he left a comment indicating that these are incorrectly relying on implementation details. Without any context, I’m inclined to agree. The fact that somewhere deep in the guts of String there lives an enum doesn’t seem important to verify. (how big it is, and how many extra inhabitants it has, does seem worth verifying longterm).
I think this was originally done by Dave Farler.
Michael
···
On Oct 26, 2016, at 5:29 PM, Alexis via swift-dev <swift-dev@swift.org> wrote:
The tests are there to ensure the reflection output doesn’t accidentally break or change. However if you’re updating the layout of those types you need to update the tests.
Slava
···
On Oct 26, 2016, at 5:41 PM, Michael Gottesman via swift-dev <swift-dev@swift.org> wrote:
On Oct 26, 2016, at 5:29 PM, Alexis via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:
I’m not clear on what the reflection tests are attempting to actually verify. Just that we don’t change the internal layout of these types accidentally? We’re churning up the layouts of a lot of the collections to get things all set up for ABI stability, which means mechanically updating these tests to expect “whatever output we now happen to output”.
In Dave’s initial eager bridging stuff he left a comment indicating that these are incorrectly relying on implementation details. Without any context, I’m inclined to agree. The fact that somewhere deep in the guts of String there lives an enum doesn’t seem important to verify. (how big it is, and how many extra inhabitants it has, does seem worth verifying longterm).
The tests are there to ensure the reflection output doesn’t
accidentally break or change.
Unless you have reason to think it will break or change *only* for some
particular stdlib types, and that reflecting the implementation details
of those types is somehow an important use-case, you can easily verify
that just as well by using some types that *aren't* in the standard
library.
···
on Wed Oct 26 2016, Slava Pestov <swift-dev-AT-swift.org> wrote:
However if you’re updating the layout of those types you need to
update the tests.
Slava
On Oct 26, 2016, at 5:41 PM, Michael Gottesman via swift-dev <swift-dev@swift.org> wrote:
On Oct 26, 2016, at 5:29 PM, Alexis via swift-dev <swift-dev@swift.org > <mailto:swift-dev@swift.org>> wrote:
I’m not clear on what the reflection tests are attempting to
actually verify. Just that we don’t change the internal layout of
these types accidentally? We’re churning up the layouts of a lot of
the collections to get things all set up for ABI stability, which
means mechanically updating these tests to expect “whatever output
we now happen to output”.
In Dave’s initial eager bridging stuff he left a comment indicating
that these are incorrectly relying on implementation
details. Without any context, I’m inclined to agree. The fact that
somewhere deep in the guts of String there lives an enum doesn’t
seem important to verify. (how big it is, and how many extra
inhabitants it has, does seem worth verifying longterm).