sammccall accepted this revision. sammccall added a comment. Thanks, this looks like exactly the right amount of magic to me :-)
================ Comment at: clangd/Context.h:91 + /// functions that require a context when no explicit context is available. + static const Context &empty(); + ---------------- as discussed offline, this could also return a value, which might make some use cases (testing) slightly cleaner. Up to you ================ Comment at: clangd/Context.h:109 + template <class Type> const Type *get(const Key<Type> &Key) const { + const ContextData *DataPtr = this->DataPtr.get(); + while (DataPtr != nullptr) { ---------------- nit: this is a for loop in disguise :-) ================ Comment at: clangd/Context.h:131 + template <class Type, class... Args> + Context derive(const Key<Type> &Key, Args &&... As) const & { + return Context(std::make_shared<ContextData>(ContextData{ ---------------- I'd find the interface (and in particular the error messages) here easier to understand if we took a `Type` by value, rather than having `emplace` semantics. If this function took a `Type`, and `TypedAnyStorage` took a `Type&&`, the cost would be one extra move, which doesn't seem too bad. Up to you, though. (If you change it, don't forget the other `derive` overload) ================ Comment at: clangd/Context.h:168 + + struct ContextData { + // We need to make sure Parent outlives the Value, so the order of members ---------------- nit: now this is private it could just be called Data ================ Comment at: clangd/Context.h:169 + struct ContextData { + // We need to make sure Parent outlives the Value, so the order of members + // is important. We do that to allow classes stored in Context's child ---------------- Is this comment still true/relevant? I thought the motivating case was Span, but Span now stores a copy of the parent pointer (and ContextData isn't accessible by it). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits