Lookup using private discriminators should still find public things.

[jrose-apple]

private and fileprivate declarations have a special "private discriminator" to keep them from colliding with declarations with the same signature in another file. The debugger uses this to prefer declarations in the file you're currently stopped in when running the expression parser. Unfortunately, the current logic didn't just prefer declarations in the current file---it only took declarations in the current file, as long as there weren't zero. The fixed version will include any declarations in the current file plus any declarations with internal or broader access.

(Really we shouldn't do this at the lookup level at all; it should just be a tie-breaker in solution ranking in the constraint solver. But that would be a more intrusive change.)

rdar://problem/27015195

---

Before merging this pull request to apple/swift repository:

• Test pull request on Swift continuous integration.

Triggering Swift CI

The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:

Smoke Testing

Platform	Comment
All supported platforms	@swift-ci Please smoke test
All supported platforms	@swift-ci Please smoke test and merge
OS X platform	@swift-ci Please smoke test OS X platform
Linux platform	@swift-ci Please smoke test Linux platform

A smoke test on macOS does the following:

1. Builds the compiler incrementally.
2. Builds the standard library only for macOS. Simulator standard libraries and
   device standard libraries are not built.
3. lldb is not built.
4. The test and validation-test targets are run only for macOS. The optimized
   version of these tests are not run.

A smoke test on Linux does the following:

1. Builds the compiler incrementally.
2. Builds the standard library incrementally.
3. lldb is built incrementally.
4. The swift test and validation-test targets are run. The optimized version of these
   tests are not run.
5. lldb is tested.

Validation Testing

Platform	Comment
All supported platforms	@swift-ci Please test
All supported platforms	@swift-ci Please test and merge
OS X platform	@swift-ci Please test OS X platform
OS X platform	@swift-ci Please benchmark
Linux platform	@swift-ci Please test Linux platform

Lint Testing

Language	Comment
Python	@swift-ci Please Python lint

Note: Only members of the Apple organization can trigger swift-ci.

[jrose-apple]

Note that the compiler itself does not perform lookup using private discriminators, and direct lookup using a discriminator still only finds things with a matching discriminator.

@jimingham or @scallanan, can you review this, both that it's the desired semantics and that the implementation looks correct?

(CI is busted right now, so I'll start tests later.)

[jrose-apple]

@swift-ci Please test

[scallanan]

I understand the distinction that's being made in your patch, but I don't understand the iterator dance that's taking place. It looks like this code

 +  auto lastMatchIter = std::find_if(results.rbegin(), results.rend(),
 +                                    [discriminator](Result next) -> bool {
 +    return
 +      matchDiscriminator(discriminator, next) == DiscriminatorMatch::Matches;
 +  });
    if (lastMatchIter == results.rend())
      return;

returns out with nothing if nothing answered "Matches;" weren't we trying to make it so we'd return out with something when something answers "NoDiscriminator?"

[jrose-apple]

Ah, the first loop says "if there's nothing in the lookup results that comes from this file specifically, leave them alone [even if they contain private results from other files]". The second loop says "given that some decls are from this file specifically, remove any decls that are specifically from another file [but leave internal and public things alone]".

[jimingham]

I made a little example with one module with two files, one having a fileprivate == and another having a private ==, and then another module that uses the first with another ==, all for the same type, and it looks like lldb expressions all get the right operators in the appropriate contexts. Plus you can use other unrelated "==" operators like for Ints and Strings. I have to poke around some more to make sure I understand what I'm seeing for sure, but it looks like this is indeed doing the right thing for lldb.

[jrose-apple]

That's what I was trying to get at with the process(…) function in the test, but testing something outside the module too is definitely a good idea. And possibly testing operators separately. Glad to hear it's working!

[jrose-apple]

@swift-ci Please test Linux platform