Skip to content

[SymbolGraph] don't filter out symbols if their availability was for a platform-agnostic target #36367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 15, 2021

Conversation

QuietMisdreavus
Copy link
Contributor

@QuietMisdreavus QuietMisdreavus commented Mar 9, 2021

Resolves rdar://74670284

When filtering symbols from the symbol graph for being unavailable, the Symbol Graph AST Walker always filtered out a symbol if the availability checking mechanism returned something that matched an outdated version. However, this would always return information for platform-agnostic targets, like Swift-language or SwiftPM-specific availability info. In the case of SwiftPM, this was because swift-symbolgraph-extract never set up the SwiftPM version, so it defaulted to 0.0.0, and caused every version comparison to fail.

This PR updates the AST Walker to only consider an availability attribute if it was for a platform-specific target, thus sparing Swift-language and SwiftPM availability from being filtered out.

@QuietMisdreavus QuietMisdreavus marked this pull request as draft March 9, 2021 18:37
@QuietMisdreavus
Copy link
Contributor Author

Marking as draft until i get a test pushed. Feel free to look over the code diff in the meantime.

@QuietMisdreavus QuietMisdreavus force-pushed the QuietMisdreavus/symbol-availability branch from 83de87e to fe4984b Compare March 10, 2021 16:17
@QuietMisdreavus QuietMisdreavus changed the title [SymbolGraph][AST] always emit symbols with Swift/SwiftPM version info [SymbolGraph] always emit symbols with Swift/SwiftPM version info Mar 10, 2021
@QuietMisdreavus QuietMisdreavus removed the request for review from DougGregor March 10, 2021 16:18
@QuietMisdreavus QuietMisdreavus marked this pull request as ready for review March 10, 2021 16:18
@QuietMisdreavus
Copy link
Contributor Author

QuietMisdreavus commented Mar 10, 2021

I had a conversation with @bitjammer about this PR. It turns out that last year (#30461), SymbolGraphGen was updated to deliberately filter out symbols which were unavailable on the current platform, with the assumption that symbol graphs were going to be generated per-target anyway, and they should have the view specific to that target. However, the way this filtering was done also caught symbols with platform-agnostic availability info in the crossfire, because the methods it used always loaded the active version, which in the case of SwiftPM was never set and defaulted to 0.0.0. I've reworked the PR, added a test, and removed the draft status.

@swift-ci Please smoke test

@QuietMisdreavus QuietMisdreavus changed the title [SymbolGraph] always emit symbols with Swift/SwiftPM version info [SymbolGraph] don't filter out symbols if their availability was for a platform-agnostic target Mar 10, 2021
@QuietMisdreavus
Copy link
Contributor Author

Linux CI reported a build error in LLDB. Trying again...

@swift-ci Please smoke test Linux platform

@QuietMisdreavus
Copy link
Contributor Author

Looks like the LLDB issue was fixed in swiftlang/llvm-project#2660. Once more, with feeling...

@swift-ci Please smoke test Linux platform

@QuietMisdreavus
Copy link
Contributor Author

The Linux builder failed while checking out LLVM:

16:12:21 ======UPDATE FAILURES======
16:12:21 /home/buildnode/jenkins/workspace/swift-PR-Linux-smoke-test/branch-main/llvm-project failed (ret=128): ['git', 'fetch', 'origin', 'pull/2660/merge:ci_pr_2660', '--tags']
16:12:21 fatal: Couldn't find remote ref pull/2660/merge
16:12:21 
16:12:21 update-checkout failed, fix errors and try again

The Windows builder failed in a concurrency test, too. Since there were some updates to that in the last couple days, i'm going to try to run a complete smoke test...

Failed Tests (1):
  Swift(windows-x86_64) :: Concurrency/Runtime/async_taskgroup_throw_rethrow.swift

@swift-ci Please smoke test

@QuietMisdreavus
Copy link
Contributor Author

QuietMisdreavus commented Mar 12, 2021

Looks like the Windows failures were disabled in #36398, and i reported the Mac failures as rdar://75317828 and i've been told that they were fixed as well. Time to try again:

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please smoke test

@QuietMisdreavus
Copy link
Contributor Author

It looks like i hit a temporary outage in Jenkins when i requested the last test...

@swift-ci Please clean smoke test and merge

@QuietMisdreavus
Copy link
Contributor Author

It looks like my test info is 404ing now? Not sure what's going on...

@swift-ci Please smoke test

@QuietMisdreavus
Copy link
Contributor Author

Linux test failed on SwiftPM?

@swift-ci Please smoke test Linux platform

@QuietMisdreavus QuietMisdreavus merged commit 5871025 into main Mar 15, 2021
@QuietMisdreavus QuietMisdreavus deleted the QuietMisdreavus/symbol-availability branch March 15, 2021 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants