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
[SR-381] API to lookup a Type given a top-level class name #42998
Comments
Is there a reason it has to be a mangled name? The qualified unmangled name produced by String(T.self) should be unique for any T. I envision us having a corresponding runtime API to parse and look up a type by (unmangled) name as well. |
We need to match NSClassFromString()/NSStringFromClass() behaviour or, more accurately, their analogues in libobjc. For Swift class names deeper than two levels (e.g. a nested class), this returns the mangled type name. See: lhoward/swift-corelibs-foundation@3949982 (I did originally propose on swift-corelibs-dev what you suggested.) |
I see, is it expected that archives will be interoperable across the objc and corelibs implementations? |
I don't speak for Apple, but my understanding is yes. cc @phausler Alternatively we limit interoperability to Foundation and single-level non-generic Swift classes. I realise that the current implementation is broken for generic classes. Really it would be nice to have:
|
Tony and I discussed early on - and cross platform archival is really important in our opinion since it is one of the defining features of Foundation. |
We might be able to get away with not letting nested classes be codable due to class encoding differentials (since id guess that those are the distinct minority) but I would like to have a story for them (even if we have to modify the darwin version to be in parity) since I know that the mangling patterns are subject to change and we should try not to rely on them. |
Yeah, the mangling might be too brittle for cross-platform and cross-version mangling, especially since we've been discussing potentially using platform-dependent manglings. It might be worth changing the ObjC Foundation behavior to reliably use demangled type names for all types. There's already a swift_getTypeName API libobjc could potentially plug in to. |
actually cross platform works so far for the places we do use it surprisingly enough (it is just our module name that is the rub) the version changes however are VERY brittle; we have had one break so far since the inception of this project already. I will follow up on the Darwin version to see what we can do for that. |
FWIW @belkadan said this in a post to swift-corelibs-dev:
This appears to contradict @jckarter's statement above that "The qualified unmangled name produced by String(T.self) should be unique for any T". |
Sorry, I meant |
I think the goal here is to have it conform to an identity transform since NSClassFromString and NSStringFromClass can round trip (and consequently then of course things that are encoded with NSKeyedArchiver can be decoded via NSKeyedUnarchiver). I have a hacky (but less hacky than reaching into the stdlib for it's private functions) way of dealing with the naming of Foundation (SwiftFoundation.NSString versus NSString etc) classes that was proposed in the discussion with Jordan. It will end up making the surface functionality work similarly enough for superficial needs but in the end it will be a behavioral difference that will eventually need to be contended with. |
It should be possible for the Swift runtime to support a round-tripping getTypeByName such that |
@phausler commit 09f143983 (I think I pushed it) removes support for looking up mangled names in NSCoding, that will still allow Foundation to work without explicitly registering the classname mappings |
Noted regarding the overhead of putting the mangling code into the runtime – however if we're going to leverage the dynamic linker for looking up classes then swift_getTypeByName() will need this. The alternative of generating a name to type dictionary at compile time is also bloat (of undefined size), although it could have the advantage of being able to lookup classes for which there are no public symbols (but I don't consider this a feature). |
We shouldn't use dlsym. That's really slow, requires the symbols to be public or at least unstripped, and doesn't work at all for JITed code. We did that as a hack to get bare-bones protocol conformance lookup working in the first Swift betas and it was awful. A better approach would be a registration table for reflectable types similar to the protocol conformance table, or perhaps just using the protocol conformance table as is, since most types will conform to at least one protocol, and we can add dummy entries for ones that don't. |
This would work for NSCoding as NSCoding/NSSecureCoding are protocols. Can you give me any hints on where to start? |
Sure, for a rough cut take a look at |
Thanks @jckarter! I will take a look. |
OK, I have something simple working that implements: @warn_unused_result
public // @testable
func _typeByName(name: String) -> Any.Type? No caching yet but that should be trivial to add. Of course it only works if the class conforms to a protocol, so it's not a general purpose solution, but it would be good enough for NSKeyedUnarchiver, or perhaps all of Foundation (given the presence of NSObjectProtocol). Edit: saw your comment above about adding dummy conformance entries for types that do not conform to protocols. |
An initial non-caching, incomplete implementation sounds like a good starting point to me. We can build from there. Thanks for taking this on! |
A pleasure and thanks for being so responsive (particularly over the holiday break!). It'll be great for object serialization to have the dynamism of the Foundation on Darwin. |
This patch is working OK with NSKeyedUnarchiver, the only issue is that subclasses will not have a protocol conformance entry. So something like: aClass = _typeByName("Foundation.NSMutableArray", conformingTo: NSCoding.self) will fail. So it's possible the only approach that will work is to have dummy conformance table entries for every class, or at least duplicate ones for subclasses, at which point maybe we are better off just emitting a table of pointers to the class metadata in each object. That might be beyond me though... |
Here's where I am so far: |
Cool, feel free to start a pull request. I'll be on real vacation starting tomorrow though so might not be able to comment till next week. IMO, the runtime call should just be |
It is not un-reasonable for us to mark NSMutableArray to adopt NSCoding if that would work. Not sure if that would help any or not. |
Unfortunately explicitly conforming subclasses to a protocol to which their superclass already conforms triggers the following error: $ ./swiftc -o lookup lookup.swift
lookup.swift:8:29: error: redundant conformance of 'SubClass' to protocol 'SomeProtocol'
class SubClass : SomeClass, SomeProtocol {
^
lookup.swift:8:7: note: 'SubClass' inherits conformance to protocol 'SomeProtocol' from superclass here
class SubClass : SomeClass, SomeProtocol {
^ If we can silence the error it would work for Foundation – but it'll be confusing for end users because the failure (not being able to unarchive) will happen at runtime. But it would be a start I guess. |
I agree we ultimately want a generic Edited: I think I see what you mean. |
I think that's fine, since only working on types with conformances is hopefully a temporary situation. That error is intended language behavior, and I don't think we can reasonably disable it. Can Foundation encode NSMutableStrings as NSStrings? Are there other important subclasses? |
We might be able to get away with disabling subclass encoding for now, I'll look at what the other classes are. If they're just mutable subclasses, it's perhaps more an uncommon scenario than their immutable superiors, and fortunately NSNumbers (which are a subclass of NSValue <NSCoding>) are encoded directly. (We can't change the encoding format, e.g. to add a mutable flag, because the format needs to be identical with Foundation on Darwin.) |
Doing the caching in the stdlib implementation is fine, I just haven't done it yet. |
In NSStringFromClass(), what's the safest way to check if a class is a generic – is it safe to look for "<" in the unmangled string representation or can that character be escaped? |
Not sure I understand the comment about registration. Are you suggesting we only need a function that provides the mangled name given a type? Or are you suggesting that Foundation only needs to support encoding, not decoding? Providing classes for a decoding session only applies to secure coding and also does not require that the caller explicitly register a class name to class mapping. Only providing encoding support runs counter to the goal of Foundation having feature parity on both platforms. |
I really think you're doing this backwards. Assume mangled names, pattern-match the one safe case, and turn that back into the fully-qualified name. That's what libobjc does (search for I don't remember how to get a mangled name for a type but I think that's in the stdlib somewhere. Or at least a bare runtime entry point. |
The runtime should handle the core type lookup logic in any case. My concerns with using mangled names is that mangled names are bad API for reflective purposes, since they're unreadable and nontrivial to compose. We're already careful about changing the format produced by |
In reply to @belkadan, there is _swift_buildDemanglingForMetadata() in stdlib which the PR for this bug report exposes as _canonicalTypeName(). OK, what you say makes sense. I'll remove _typeByName() and put the transformation for the one safe case into Foundation. |
@jckarter two issues with unmangled names:
|
How about this then: private func buildCanonicalNameForClass(aClassName: String) -> String? {
var name : String
if aClassName.hasPrefix("_Tt") {
return aClassName
}
var components = aClassName.bridge().componentsSeparatedByString(".")
if components.count == 1 {
components = [ _SwiftFoundationModuleName, aClassName ]
} else if components.count != 2 {
return nil
}
if components[0].isEmpty || components[1].isEmpty {
return nil
}
name = "_TtC"
if components[0] == "Swift" {
name += "Ss"
} else {
name += String(components[0].length) + components[0]
}
name += String(components[1].length) + components[1]
return name
}
internal func NSClassFromString(aClassName: String) -> AnyClass? {
guard let canonicalName = buildCanonicalNameForClass(aClassName) else {
return nil
}
return _typeByCanonicalName(canonicalName) as? AnyClass
} |
> Providing classes for a decoding session only applies to secure coding If we only supported secure coding for Linux (at least for now), this would be good enough to build a list of all possible class names you could encounter in the archive. I'm not saying this is an ideal solution, but it gets Foundation unblocked while we (Foundation and Swift runtime) design and figure out @jckarter and I talked this morning for quite a while about mangled vs. demangled names, and didn't quite manage to convince each other either way. Things that came up:
|
Just to address the first comment:
|
Thanks for the analysis above, it all makes sense. The only thing I would add is: libobjc/Foundation on Darwin uses a particular encoding of class names today (unmangled for top-level classes, mangled otherwise). Corelibs Foundation has an explicit goal of compatibility with Foundation on Darwin, as I understand it. So whilst I understand the desire for a long-term solution perhaps we need something that matches libobjc's behaviour in the interim, with whatever division of responsibility between stdlib and Corelibs Foundation deemed appropriate. |
Yep, sorry, hadn't kept up. Thanks for the new implementation! > "All possible class names" is unbounded. …aargh, I forgot that the root object does not itself have a set of expected classes. |
I added a cache but it uses StringMap which pulls in libLLVMSupport (and libswiftCore.dylib seems to be linked with -all_load). Not ideal: we should probably use a different implementation or somehow pull in StringMap.cpp directly. |
We shouldn't link the runtime against any LLVM libraries. Can you use DenseMap instead, or the ConcurrentMap that's implemented in the runtime already? |
Fixed to use ConcurrentMap. |
Until such time we have a stable and unique de-mangled name format: public // SPI(Foundation)
func _topLevelClassByName(name: String) -> AnyClass? Edit: we could just call this typeByName and return Any but I think that is misleading, even though that's the eventual API we want. I have updated the nscoding branch of Foundation to use this. I am still cleaning up the pull request for stdlib with a view to requesting it to be integrated after the 2.2 branch. |
NSKeyedArchiver/NSKeyedUnarchiver Linux/OS X tests pass with this change. |
One more thing: in the interests of minimizing developer surprise, we should find a way for this API to work where the class has no direct protocol conformance. If emitting dummy conformance records for every type is problematic, we could special case NSCoding. (In the Swift 3 timeframe perhaps a different approach to runtime metadata will exist, but for the interim.) Foundation is using an explicit dummy protocol for codable classes that inherit NSCoding, but expecting developers to know the limitations of NSClassFromString might be unreasonable. |
We can add that later. Developer surprise isn't a problem for a work-in-progress. |
Here is a patch to include type metadata in a new section, in the case where it is not already included in the protocol conformance table. https://github.com/apple/swift/pull/834/files I also (per an earlier comment from @jckarter) reverted the API consumed by Foundation to |
Merged to master. |
Foundation NSKeyedArchiver/NSKeyedUnarchiver tests pass on Linux with swift master (thanks to @gribozavr for fixing build regression I accidentally introduced!). |
Comment by admin (JIRA) (updating fields only) |
Additional Detail from JIRA
md5: 13f1c2ae7174aab8983f1dde3a976c53
blocks:
relates to:
Issue Description:
This may be useful to implement NSStringFromClass()-compatible behaviour which is necessary in order to implement NSKeyedArchiver. I am using the attached workaround at the moment but obviously it is very fragile.
The text was updated successfully, but these errors were encountered: