Found a bug in Foundation's obsolescence mechanism in Linux

Development Core Libraries
, ,
1

I'll be quick. Here's an outline:

  • I'm porting a library to Linux, SourceKittenDaemon.
  • The library used to compile in September, after Swift 4.0 was released.
  • Now it doesn't. The compiler says that our use of NotificationCenter.default.addObserver is ambiguous. Here's the console output:
Compile Swift Module 'SourceKittenDaemon' (8 sources)
/home/felix/Documents/Programming/SKD/SourceKittenDaemon/Sources/SourceKittenDaemon

/home/felix/Documents/Programming/SKD/SourceKittenDaemon/Sources/SourceKittenDaemon/Completer/Completer.swift:98:28: error: ambiguous use of 'addObserver'
        NotificationCenter.default.addObserver(
                           ^
Foundation.NotificationCenter:9:15: note: found this candidate
    open func addObserver(forName name: Foundation.NSNotification.Name?, object obj: Any?, queue: Foundation.OperationQueue?, usingBlock block: @escaping (Foundation.Notification) -> Swift.Void) -> NSObjectProtocol
              ^
Foundation.NotificationCenter:10:15: note: found this candidate
    open func addObserver(forName name: Foundation.NSNotification.Name?, object obj: Any?, queue: Foundation.OperationQueue?, using block: @escaping (Foundation.Notification) -> Swift.Void) -> NSObjectProtocol
              ^
error: terminated(1): /home/felix/Documents/Programming/Binaries/swift/swift-4.0.3-RELEASE/usr/bin/swift-build-tool -f /home/felix/Documents/Programming/SKD/SourceKittenDaemon/.build/debug.yaml main

Here's the thing. This problem is strange, because both functions' signatures are basically the same. I looked up the class, and indeed, both functions exist, but one of them is marked as obsolete:

This file in the corelibs repo: file

    @available(*,obsoleted:4.0,renamed:"addObserver(forName:object:queue:using:)")
    open func addObserver(forName name: NSNotification.Name?, object obj: Any?, queue: OperationQueue?, usingBlock block: @escaping (Notification) -> Void) -> NSObjectProtocol {
        return addObserver(forName: name, object: obj, queue: queue, using: block)
    }

    open func addObserver(forName name: NSNotification.Name?, object obj: Any?, queue: OperationQueue?, using block: @escaping (Notification) -> Void) -> NSObjectProtocol {
        let object = NSObject()
        
        let newObserver = NSNotificationReceiver()
        newObserver.object = object
        newObserver.name = name
        newObserver.block = block
        newObserver.sender = _SwiftValue.store(obj)
        newObserver.queue = queue

        _observersLock.synchronized({
            _observers.append(newObserver)
        })
        
        return object
}

This problem tells me that somehow the compiler doesn't know that the function is obsolete, and when trying to fit a function to our code finds two candidates and reports an error.

Maybe this only happens on Linux? I've heard from our Mac users that SKD is compiling perfectly on their machines, so that could be it.

Anything I can do to help, please tell me.
Here's the Github thread for if you want more info / try getting the error yourselves.

1 Like
2

That definitely sounds like a bug to me. There are a few options to fix it: first and easiest avoid the trailing block syntax (that should disambiguate the calls and be portable), we could delete the obsolete function, or the better option here is we could change the way ambiguity is resolved to favor non obsolete functions (this seems like the best long term solution but probably harder)

1 Like
3

An immediate source-compatible fix would be to disable trailing closure syntax selectively for the obsolete function by tacking on a defaulted Void parameter. That way, any explicit use of the obsolete function still works and prompts the warning, and any use of trailing closure syntax becomes unambiguous.

    @available(*,obsoleted:4.0,renamed:"addObserver(forName:object:queue:using:)")
    open func addObserver(forName name: NSNotification.Name?, object obj: Any?, queue: OperationQueue?, usingBlock block: @escaping (Notification) -> Void, _: Void = ()) -> NSObjectProtocol {
        return addObserver(forName: name, object: obj, queue: queue, using: block)
    }
3 Likes
4

Xiaodi, :thinking: that sounds like the best option to me. How can I get it added? Should I just issue a PR? Maybe I should file a JIRA report as well?

5

Sounds reasonable to me!

1 Like
6

For things like these, you can send a PR; if you won’t be able to, writing a bug on Jira is the best way to ensure it is kept track of.

1 Like
7

Okay, I'm doing this as we speak. I've compiled swift together with Foundation (build-script --foundation).

I want to test if the modified version of Foundation changes the behavior of the compiler. I have swift installed in this computer. Will the recently built compiler use the Foundation I have installed, or will it use the Foundation I've just compiled?

8

Generally speaking, the compiler does not use Foundation. You can add your library to the path — use 'ninja test' in the Foundation repository to get a full command line you can use to direct Swift to the compiled Foundation library. (You may have to reconstruct what build.ninja does to compile TestFoundation to compile your own executables with the new Foundation library.)

9

I have terrible internet, so this is what I’m able to fish out from my cellphone for today:

  • I did the ninja test and got a suggestion for LD_LIBRARY_PATH. It didn’t work, but I think that’s because either (a) I need another library that I haven’t compiled, or (b) since I’m trying to use the master branch-compiled Foundation for linking, but using my 4.0.3-release swiftc binaries for compiling the project, the 4.1 and 4.0 -made binaries clash. That would make a lot of sense since the ABI isn’t stable, right?

  • What’s the common workflow for testing how changes in Foundation affect swift projects?

Thanks for the help :hugs:

10

(Now I'm somewhere else with decent internet)

I think the problem I was having is that swift build didn't work. Of course: I didn't build SPM. Now I'm doing that, let's see if I have everything necessary now.

11

Sorry for the noobness.

The ninja test command passes and all, so @xwu's fix doesn't break anything. What I'm trying to test now, is if it fixed the bug in Foundation.

What I wanna do

Invoke swift build with the patched Foundation binary in the linking path. If I can, then I can test if the patch fixes this bug.

Steps

That's done (afaik) by adding the directory where libFoundation.so exists to LD_LIBRARY_PATH.

But I also need to compile SPM, because the fix should land at the earliest in Swift 4.1 land... right? (If I'm wrong, this should be easier actually).

This is what I'm doing

I've compiled Swift with the following command:

./swift/utils/build-script -b -p --xctest --foundation --jobs 1

Then, I'm invoking SPM inside the SourceKittenDaemon directory by using the following command:

../../build/Ninja-DebugAssert/swiftpm-linux-x86_64/debug/swift-build

But SPM fails with this error:

felix@felix-X550LD ~/D/P/S/F/S/SourceKittenDaemon> ../../build/Ninja-DebugAssert/swiftpm-linux-x86_64/debug/swift-build
unable to restore state from /home/felix/Documents/Programming/SwiftPRs/FoundationBugJan2018/SKD/SourceKittenDaemon/.build/dependencies-state.json; unsupported schema version 1
/home/felix/Documents/Programming/SwiftPRs/FoundationBugJan2018/SKD/SourceKittenDaemon: error: manifest parse error(s):
/home/felix/Documents/Programming/SwiftPRs/FoundationBugJan2018/SKD/SourceKittenDaemon/Package.swift:1:8: error: no such module 'PackageDescription'
import PackageDescription
       ^

What am I doing wrong?

Can I test this in Swift 4.0.3? That is, by just recompiling Foundation and replacing my Foundation binaries? I'd test it, but I prefer to ask first because in my computer, the compiler takes an entire night to build.

12

Okay, nevermind.

I’m compiling the 4.0.3 tag repos. Those are the binaries I have installed, and the ones I’m building should be compatible with my toolchain.

If they are, then I can just swap in the patched, 4.0.3 Foundation binary and see if it fixes the bug. Then we’ll be set for a PR.

13

IT WORKS.

Compile Swift Module 'SourceKittenDaemon' (8 sources)
/home/felix/Documents/Programming/SwiftPRs/FoundationBugJan2018/SKD/SourceKittenDaemon/Sources/SourceKittenDaemon/Completer/Completer.swift:35:23: warning: result of call to 'watch(paths:for:thenInvoke:)' is unused
            myWatcher.watch(
                      ^    ~
/home/felix/Documents/Programming/SwiftPRs/FoundationBugJan2018/SKD/SourceKittenDaemon/Sources/SourceKittenDaemon/Completer/Completer.swift:98:36: warning: result of call to 'addObserver(forName:object:queue:using:)' is unused
        NotificationCenter.default.addObserver(
                                   ^          ~
Compile Swift Module 'sourcekittend' (2 sources)
Linking ./.build/x86_64-unknown-linux/debug/sourcekittend

I can make the PR now.

Thanks y'all!

14

Please check [SR-6419] `NotificationCenter.addObserver` of SCLF with trailing closure syntax cannot be used on Swift 4.0.2 · Issue #3786 · apple/swift-corelibs-foundation · GitHub. The problem here was the inappropriate usage of @available(*, obsoleted: 4.0) and that had been addressed in [SR-6419] Use `@available(*, unavailable)` instead of inappropriate `@available(*, obsoleted: 4.0)` by ikesyo · Pull Request #1323 · apple/swift-corelibs-foundation · GitHub already.

15

Yes! I’ve just answered to you in GitHub (you beat me to it here! ^^). Wanna continue here or do you prefer there?

16

Okay, this was already solved. Here's what happened:

  • The bug appeared in 4.0.X and there I saw it.
  • It was caught and fixed already for 4.1
  • I just tested it and it works in the snapshots.

Thanks @ikesyo for pointing the original bug and patch! And thanks to @xwu and everyone for their help and patience.

The issue is now closed :)

2 Likes