[Reflection] Support OpaqueExistential in RecordTypeInfo::readExtraInhabitantIndex

[kastiglione]

Support OpaqueExistential in RecordTypeInfo::readExtraInhabitantIndex().

This missing support was identified when calling projectEnumValue() with a type of Any?. In that case, the projectEnumValue was returning false because readExtraInhabitantIndex returned false. For OpaqueExistentials, readExtraInhabitantIndex should look at the type field (named "metadata"), and use that to determine extra inhabitants.

Unblocks rdar://68171421

[kastiglione]

This is an initial working version for macOS x86-64, in a specific case I was debugging in lldb. Still todo are:

• Verify other platform support (!macOS || !x86-64)
• Verify opaque existential + nested optional (Any?? etc)

[kastiglione]

@swift-ci test

[tbkka]

Nice! This is simpler than I thought it would be. (To be honest, I've not looked at this code in a little while...). I see a couple of other unimplemented cases that we might be able to fill in fairly quickly.

I'll sketch out some test cases this afternoon to help push this forward.

[kastiglione]

test cases sound great, thanks

[kastiglione]

I've changed the base branch to main now, as requested.

[kastiglione]

I'm hoping to merge this soon, and then to cherry pick it to release/5.4. Are you ok with doing the other cases in a separate change because of this? I can add some tests for this case. Based on #30635, is validation-test/Reflection still the place to write reflection tests?

[kastiglione]

@swift-ci test

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 340b85f

[tbkka]

Ugh. Looks like I made a lot of obvious dump copy/paste errors in the 32-bit version. I'll work on that this afternoon...

[tbkka]

I'm surprised that Linux passed here. I'll have to double-check, but I thought Linux only had 4k extra inhabitants in 64-bit pointers, where macOS has 4G.

[tbkka]

@swift-ci Please test macOS

[kastiglione]

I did some fixes in 7ed7122, and I'll run the tests locally to iterate on them faster.

[kastiglione]

I see we're both on it haha

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 7f8bebb

[kastiglione]

looking

[tbkka]

@swift-ci Please test macOS

[tbkka]

This looks right to me. "Just" need to finish filling out the test suite for it.

Thanks for pushing this forward...

[kastiglione]

This looks right to me.

Thanks!

"Just" need to finish filling out the test suite for it.

@tbkka Are you intending for "filling out the test suite" to be done in this PR or in a follow up?

Thanks for pushing this forward…

thanks for your guidance.

[kastiglione]

@swift-ci test Linux

[tbkka]

Sorry, I probably should have said "get this test suite to pass." I think the test we have is sufficient coverage, but it may still have some warts. More tests can follow in a subsequent PR, I think.

[kastiglione]

The new test you wrote pass for the different architectures:

PASS: Swift(macosx-x86_64) :: Reflection/reflect_Optional_Any.swift (678 of 14039)
PASS: Swift(iphonesimulator-i386) :: Reflection/reflect_Optional_Any.swift (679 of 14039)
PASS: Swift(iphonesimulator-x86_64) :: Reflection/reflect_Optional_Any.swift (679 of 14039)
PASS: Swift(watchsimulator-i386) :: Reflection/reflect_Optional_Any.swift (679 of 14039)

[tbkka]

Removed swift5-branch-managers from reviewers list; you don't need their review for merging into main

[davezarzycki]

@rjmccall and @jckarter – Is there any reason why different Linux platforms would have different extra inhabitant counts? This new test isn't passing on my Fedora 33 (x86-64) box and the test seems to assume that the maximum number of extra inhabitants for 64-bit platforms are all the same (which isn't true).

[davezarzycki]

Now I'm wondering if the tests are executed on the Linux build bots at all (as opposed to just my "unified build" setup). Let's find out: #35512

[davezarzycki]

Ding. This test isn't triggering on the Linux bots but it is on my cmake / unified build:

$ curl https://ci.swift.org/job/swift-PR-Linux/25240/consoleText 2>/dev/null|grep reflect_Optional_Any
UNSUPPORTED: Swift(linux-x86_64) :: Reflection/reflect_Optional_Any.swift (617 of 14002)

I'm going to commit a workaround that unbreaks unified builds: #35513

[tbkka]

@swift-ci Please test Linux

[tbkka]

On Linux, pointers only have 4096 extra inhabitants. that requires some finessing of the test. I'll work on that.

[davezarzycki]

Thanks. Do you think you'll have a fix soon or should I merge the workaround?

[adrian-prantl]

Ding. This test isn't triggering on the Linux bots but it is on my cmake / unified build:

Just in case this is something obvious — AFAIK the reflection tests are part of the validation tests. Is your bot running these?

[davezarzycki]

Yes, my bot always runs the validation tests. (They just take a few seconds.) I thought some of the Swift bots run the validation tests. Is that still true?

[tbkka]

I thought that "swift-ci test" ran validation tests "swift-ci smoke test" did not. But I may have misunderstood.

Please merge a workaround for now. I may not get a fix ready until tomorrow.