Namespaces in API notes

Looks like this PR by @zoecarver was reverted due to a C++ compile-time regression. I’m not well versed enough in clang to understand why (or even, why this would affect C++ compilation outside of Swift importing C++ APIs), is this a fundamental issue with the approach or is this something that can be fixed in the near future?

Being able to add API notes to namespaces types is blocking me from building a more complete prototype of importing MLIR APIs into Swift. It seems like the most significant issue may be related to nested namespaces and tags? Would it be possible to merge partial support for only a top-level namespace? (All of the types I would need to annotate would be directly under the mlir namespace, most notably attaching import_as_ref to mlir::MLIRContext).

The compile time regression should be pretty simple to fix. I think it was only effecting code that used API notes, but I am not sure. The issue is, Objective-C doesn't (really) support nested, named decls. So, API notes would just visit every top-level decl looking for attribute to apply. For methods, it would iterate over the children of an Objective-C class, again looking for attributes to apply.

Well, we took the same approach for C++, when we saw a namespace or record, we'd look through its members. Somehow we ended up recursively reading a lot of members. I think maybe members of a namespace are technically global and we'd somehow visit each member of a namespace per each member of a namespace or something? Anyway, it was quadratic and causing relatively simple programs to take hours to compile. The solution is: we need to just make sure that we visit each decl once.

That being said... you can now use move only types like unique_ptr, so maybe you don't need API notes quite as much? (Until this patch lands, you need to still pass -Xfrontend -enable-experimental-move-only: Support `std::unique_ptr`. by zoecarver · Pull Request #65577 · apple/swift · GitHub)

Thanks for following up on this. I'm still unable to use the MLIRContext via C++ interop on swift-DEVELOPMENT-SNAPSHOT-2023-05-14-a. I've put together an example here: GitHub - GeorgeLyon/circt-swift-interop: Repository tracking how upstream changes to CIRCT/MLIR affect Swift interop, details are in the README

I think MLIRContext is non-copyable and non-movable, since it just deletes its copy constructor and doesn't define a move constructor. I think move-only Swift types can't represent such type in that case, isn't that right @zoecarver . In that case, you would need to import it as a foreign reference type. Could we do such an import automatically I wonder.

We should be able to import non-copyable and non-movable types. We just have to make sure we never move them. We basically just have to make sure we always borrow them (or mutably borrow them with inout). This shouldn't be too hard because C++ has to do this anyway. The hard part will be re-writing methods that return a reference

MLIRContext &getCtx();

As Swift functions that take a closure

func withGetCtx((borrowing MLIRContext) -> Void)

Same with initializers. In other words, a borrowed move only type will never copy, so we just have to make sure non-movable types are always borrowed. The short term solution can be to just enforce this rule, without re-writing. That should allow you to make progress.

I figured this, which is unfortunate because constructors are one of the things I want to avoid re-writing in Swift. Weird that it doesn’t recognize the type though.

I wonder if that can be expressed as something like ~Consumable on the Swift side, similar as to how ~Copyable expresses lack of copyability.