[SR-381]: API to lookup a Type given a top-level class name #834

lhoward · Dec 31, 2015

Implement a _typeByName(name: String) -> Any.Type? function, currently explicitly limited to top-level classes.

Type metadata for types that do not explicitly conform to a protocol is emitted in a new section, .swift2_type_metadata. The runtime searches both this section and the protocol conformance table.

Whilst implemented on top of an internal API that looks up a type given a mangled name (in order to assure that we are indeed looking up a class), no public Swift APIs are defined that expose the name mangling format.

This is to support [NS-377] Foundation component NSKeyedUnarchiver.

Update: separate generic instantiation into a separate pull request.

You can pass an llvm::StringRef down instead of copying the string into a std::string here.

Maybe we can factor installCallbacksToInspectDylib into the constructor on Conformances so that it happens as part of initialization of the conformance cache state. That way it doesn't need to be manually called on both the conformsToProtocol and getTypeByName paths.

Is this TODO still relevant?

Thanks, removed

You should also note that this doesn't handle generic or non-nominal types yet (which is fine, for a start).

This change makes installCallbacksToInspectDylib() deadlock when Conformances.get() is called from swift_registerProtocolConformances(), I think because Lazy hasn't initialised yet.

Interesting, we've had reports of similar deadlocks with the code as it is today that might have the same underlying cause.

I left installCallbacksToInspectDylib() in both places for now, as it deadlocks when called from the Conformances constructor because the Lazy container is still being initialised.

Sounds good.

You should pass StringRef by value, not by const reference.

Thanks, fixed (sorry rookie C++ programmer here)

In d0be84e I refactored the conformance cache initialization to include installing the callbacks without deadlocking (that I could see).

Saw that, rebased against master and building now.

Should we have a SWIFT_OBJC_INTEROP case that calls objc_lookUpClass()? Is it safe to static cast a Class to a Metadata or does it need to be wrapped?

Indentation is 2 spaces.

Please add tests.

Is this really @testable, or is it actually SPI(Foundation)?

Fixed

Accidental commit of this, haven't tested on OS X yet. Is this desired functionality?

Beginnings of tests – confirmed passing

This API only works with types. AFAICS there's no reason to require or check for the _Tt prefix.

Fixed

Metadata patterns for generic types are in the protocol conformance table too. We shouldn't rely on the symbol table for anything.

I couldn't find a way to get the pattern name from the conformance table (even the existing ProtocolConformanceRecord::dump() uses dladdr()). Can you give me a pointer on where to find it? We need the pattern name to match the corresponding component of the name being resolved.

It's a bit hokey to get at it, but there should be a pointer to the nominal type descriptor somewhere in the pattern itself. It might be worth adding a more direct name reference to the template somewhere.

I haven't found it yet but I will keep looking :)

We'd only ever emit type metadata records for types in the current object file, so the indirectability isn't necessary. There should also be records for generic metadata patterns.

We support generic types by registering them from swift_getGenericMetadata(), although I suppose we could emit a reference to the type metadata accessor for the specialised type instead, in case it hasn't been instantiated yet. Is that what you were suggesting?

If you mean patterns that are specialised dynamically by the name being looked up, I wasn't sure how to do this and also validate any protocol requirements on the type. I'm probably a bit confused...

Will remove indirectable types.

Where are you seeing this fail?

Only on Linux with an Optional of MetadataKind::Opaque. I didn't investigate further as _swift_buildDemanglingForMetadata() suggests this is expected.

I see, seems reasonable.

Can this be static? It doesn't look like it's used outside of this file.

_registerTypeMetadata() is called when patterns are instantiated by the type metadata accessor for the specialised type (see comment above).

I don't think that's a good approach. The table should statically reference the generic type metadata pattern, and the runtime can use that to instantiate a generic type when needed.

I did originally implement that approach but it blew up when you tried to instantiate a generic type where the arguments had type requirements. Is it useful to dynamically specialise a type given a name? And if so, how do you do it safely?

Or – by statically referencing the type metadata pattern do you mean referencing the metadata accessor for a specialised type? (i.e. the code emitted by the compiler that calls swift_getGenericMetadata())

Given some specialised name N where some of the pattern arguments have type requirements, I'm not sure how you get (and correctly order) the witness table references that are ultimately passed by swift_getGenericMetadata() to the pattern's CreateFunction().

Let's say I want to instantiate Dictionary at runtime. I can find the pattern for Dictionary, and the type metadata for String. But I also need the protocol witness table for Hashable, and moreover, I don't know I need that given a name like "GVs10DictionarySSSS_". What am I missing?

Referring to accessor functions from these tables is ultimately the right resilient solution for both nongeneric and generic types. I feel like we should do that as a separate stage though. For now, all the information you need to form a getGenericMetadata call from a pattern ought to be available from its nominal type descriptor (though even that's hard to recover, as you've discovered).

OK. I guess I need to do some more digging.

I still say it's fine not to support generics for now.

Sure. If I can't figure out how to get the NTD and witness tables from a pattern I'll remove generic support. (After all the public API is limited to top level classes only.) But it'd be nice to get the plumbing complete from the start.

…

So – and thanks for your patience with me as I navigate around this code – are you suggesting we emit generic pattern metadata records for unbound generic types (so that they can be bound when _typeByName() is called)? Or only for types that have been specialised at compile time?

If the latter, wouldn't it be simpler to emit references to the type metadata accessor for the specialised type rather than the pattern?

If the former then, OK, I will keep digging :)

It might be best to do nothing for generic types for now. I think we're going to be changing how this all works soon.

Another question: when/where would a bound generic type be registered with the ObjC runtime? Can we employ a similar strategy for the non-ObjC case?

Re: removing generic type support, OK.

The generic metadata pattern's instantiation function handles that when you call getGenericMetadata.

So, playing devil's advocate: why not register the name to metadata mapping for an instantiated generic type from getGenericMetadata()? (This is what my current approach does. I'm just not clear whether there are cases where you'd want to be able to lookup a bound generic type by name and it had not already been otherwise already instantiated by the runtime.)

That means every generic instantiation would immediately warm up the type lookup cache, when it could be done more lazily. It also adds more memory overhead to every generic instantiation. Type lookup isn't a fast path, so we should optimize for memory efficiency. As long as name lookup can eventually find enough artifacts to instantiate a generic, whether that be the pattern or an accessor function, that should be sufficient.

Thanks for the explanation Joe. I think using the accessor function would be simpler, I was concerned that one wouldn't be emitted if you didn't create an instance of the specialised type but:

class MultiGenericClass<T : Equatable, U> {}
var someType = bar() as? MultiGenericClass<String, Int>.Type

does appear to emit it.

I'll push to remove the generic support and if I can find an easy way to emit a reference to the accessor function for generics as an additional patch, I'll do that. Thanks again for taking the time to explain all of this to me.

jckarter · Jan 11, 2016

I believe getsectiondata is expensive on Darwin. I think it might be better to collect the type metadata and conformance records under one section/symbol and register them together than to keep them separate.

lhoward · Jan 11, 2016

I can take a look at coalescing the two once the rest is fixed. Maybe it's simpler to encode the type metadata record into a protocol conformance record (even though it wastes a word or two).

jckarter · Jan 11, 2016

Let's keep them separate for now.

lhoward · Jan 12, 2016

@jckarter – OK, I have some code working to dynamically instantiate generic types. (I figured out how to get the nominal type descriptor from the pattern.)

Where I am stuck is – for generic patterns with associated type requirements, how do I get the corresponding protocols? If I can get these, I can call swift_conformsToProtocol() to validate the dynamic pattern and grab the witness tables. I'm sure I'm missing something obvious as usual!

I note that struct GenericParameterDescriptor has a note about including protocols in the parameter array.

lhoward · Jan 12, 2016

I want to do:

  for (size_t i = 0; i < ntd->GenericParams.NumPrimaryParams; i++) {
    auto metadata = findMetadataForNode(typeListNode->getChild(i));
    arguments[i] = const_cast<void *>(reinterpret_cast<const void *>(metadata));
    if (arguments[i] == nullptr) {
      delete arguments;
      return nullptr;
    }

    for (size_t j = 0;
         j < ntd->GenericParams.Parameters[i].NumWitnessTables; j++) {
      void **witnessPointer = &arguments[witnessIndex + j];
      // if there are associated type requirements then check this argument conforms
      // and slam in the witness tables for the requirement  
    }

    witnessIndex += ntd->GenericParams.Parameters[i].NumWitnessTables;
  }

  auto metadata =
    swift::swift_getGenericMetadata(const_cast<GenericMetadata *>(generic),
                                    arguments);

lhoward · Jan 12, 2016

OK, I got lookup of generics with associated type requirements working by extending GenericParameterDescriptor to include the ProtocolDescriptors for any type constraints that require witness tables. See the comment in Metadata.h for more information.

jckarter · Jan 13, 2016

Can you separate the generic instantiation stuff into its own pull request, so we can get the basic nongeneric functionality in?

lhoward · Jan 13, 2016

Done.

jckarter · Jan 13, 2016

Thanks!

Instead of the typedef here, can we update uses of ProtocolConformanceTypeKind to use the new name?

Fixed

This doesn't need to be indirectable. We'll only ever emit these records for types in the current object file.

This is pretty inefficient. You should be able to ask a NominalTypeDecl what protocols it conforms to. You'll also have to filter out AnyObject and @objc protocols, which we don't emit Swift protocol conformance records for.

Fixed

I think String has a withCString method that might be more efficient if the String is using ASCII or UTF-8 storage. cc @gribozavr

Did not change this yet.

Is this still necessary?

Will remove swift_getMangledTypeName().

Is Remangle.cpp still necessary in the non-ObjC-compatible runtime?

Will remove swift_getMangledTypeName().

lhoward · Jan 15, 2016

Because there is no pattern support in this branch, there is no support for resilient non-generic type lookup. I can fix this but it will involve back-porting some code from the generic branch, but I will keep it as a separate commit in case you wish to back it out.

jckarter · Jan 15, 2016

No problem. Let's get the basic case working to unblock Foundation.

lhoward · Jan 15, 2016

Ah yes. I forgot why I was doing this originally. ;)

lhoward · Jan 15, 2016

Patched committed for non-generic resilient support.

…

lhoward · Jan 15, 2016

~~Argh. I've broken something and I can't build Foundation without getting duplicate symbol errors (like __TFV15SwiftFoundation18NSTextCheckingTypeg8rawValueVs6UInt64).~~

Seems to have disappeared on both branches, all I did was a rebase of swift against master and rebuild. I will investigate more tomorrow to see if I can figure out exactly what the cause was. Validating on Linux should be a good confirmation all is well.

Update: all good on Linux. Heisenbug I guess.

jckarter · Jan 15, 2016

I appreciate your enthusiasm, and thanks for bearing with my nitpicks, but continuously adding functionality makes it hard to converge on review here. The metadata structures are in a state of flux too, so I'm wary of building too much on what we have now. I'd like to focus on just making sure the corelibs' work to get to parity with Darwin is sufficiently unblocked for now. If you add any new functionality, it'd be helpful if you do so in separate pull requests. Thanks!

Might want to early-exit return false here if getAnyNominal returns null.

Fixed, I think we want to return true (non-nominal types should not have a type metadata record?). Arguably typeHasExplicitProtocolConformance() should be renamed to something like typeDoesNotNeedTypeMetadataRecord(). Or the nominal check done by the caller.

OK, I think having the caller do the check and changing the signature to hasExplicitProtocolConformance(NominalTypeDecl *decl) is better.

lhoward · Jan 15, 2016

Re: general comment, understood. I did keep the resilient top-level class support in a separate commit in this PR. I'm still learning about Swift and the compiler internals, so part of the issue from my end is that I didn't really understand resilient (non-generic) types until yesterday – but when I did, I thought that it should be in scope for top-level class-by-name resolution.

lhoward · Jan 15, 2016

Thanks for your patience! 😬

jckarter · Jan 16, 2016

Looks good, thanks! Merged in 884a352.

lhoward changed the title from [SR-381]: initial _typeByName(conformingTo:) API implementation to [SR-381]: initial _typeByName() API implementation Dec 31, 2015

jckarter self-assigned this Dec 31, 2015

lhoward changed the title from [SR-381]: initial _typeByName() API implementation to [SR-381]: API to lookup a Type given a mangled name Jan 6, 2016

lhoward changed the title from [SR-381]: API to lookup a Type given a mangled name to [SR-381]: API to lookup a Type given a top-level class name Jan 8, 2016

lhoward referenced this pull request Jan 13, 2016
Closed
[SR-589] Plumbing for dynamic instantiation of generic types #959

lhoward allow name lookup to work with resilient types 2723663

jckarter closed this Jan 16, 2016

lhoward referenced this pull request Jan 16, 2016
Merged
[SR-558] Only build _swift_buildDemanglingForMetadata() for ObjC #990

+    +          foundMetadata = metadata;
+    +          break;
+    +        }
+    +      }
+    +    }
+    +  }
+    +
+    +  pthread_mutex_unlock(&C.SectionsToScanLock);
+    +
+    +  return foundMetadata;
+    +}
+    +
+    +extern "C"
+    +const Metadata *
+    +swift_getTypeByName(const char *typeName, size_t typeNameLength) {
+    +    std::string name(typeName, typeNameLength);

@@ -2755,6 +2755,50 @@ swift::swift_conformsToProtocol(const Metadata *type,
        goto recur;
      }
+    +/// Return the type metadata for a given type name.
+    +///
+    +/// This is a temporary implementation to support some Foundation use cases;
+    +/// it will only lookup a type that directly implements a protocol as only
+    +/// those types will have conformance table entries.
+    +static const Metadata *
+    +_metadataForName(const std::string &typeName) {
+    +  auto &C = Conformances.get();
+    +  const Metadata *foundMetadata = nullptr;
+    +
+    +  installCallbacksToInspectDylib();

@@ -83,6 +83,23 @@ func _typeName(type: Any.Type, qualified: Bool = true) -> String {
          input: UnsafeBufferPointer(start: stringPtr, count: count))
      }
+    +@_silgen_name("swift_getTypeByName")
+    +func _typeByNameImpl(
+    +    name: UnsafePointer<UInt8>,
+    +    _ nameLength: UInt)
+    +  -> Any.Type?
+    +
+    +/// Returns a metatype given a demangled qualified name and a protocol.
+    +/// TODO allow P to be nil; this will require a compiler change

+    +/// it will only lookup a type that directly implements a protocol as only
+    +/// those types will have conformance table entries.
+    +static const Metadata *
+    +_metadataForName(const std::string &typeName) {
+    +  auto &C = Conformances.get();
+    +  const Metadata *foundMetadata = nullptr;
+    +
+    +  installCallbacksToInspectDylib();
+    +
+    +  pthread_mutex_lock(&C.SectionsToScanLock);
+    +
+    +  unsigned sectionIdx = 0;
+    +  unsigned endSectionIdx = C.SectionsToScan.size();
+    +
+    +  // TODO: implement typeName->metadata cache
+    +  // TODO: support looking up types for which there is no conformance entry

@@ -2755,6 +2755,51 @@ swift::swift_conformsToProtocol(const Metadata *type,
        goto recur;
      }
+    +/// Return the type metadata for a given type name.
+    +///
+    +/// This is a temporary implementation to support some Foundation use cases;
+    +/// it will only lookup a type that directly implements a protocol as only
+    +/// those types will have conformance table entries.
+    +static const Metadata *
+    +_metadataForName(const llvm::StringRef &typeName) {
+    +  installCallbacksToInspectDylib();

apple/swift

[SR-381]: API to lookup a Type given a top-level class name #834

Projects

Labels

Milestone

Assignees

3 participants

[SR-589] Plumbing for dynamic instantiation of generic types #959

[SR-558] Only build _swift_buildDemanglingForMetadata() for ObjC #990