ioeric added inline comments.
================ Comment at: clangd/ExpectedTypes.cpp:27 + return llvm::None; + auto *VD = llvm::dyn_cast<ValueDecl>(R.Declaration); + if (!VD) ---------------- maybe add a comment what `ValueDecl` covers roughly? E.g. functions, classes, variables etc. ================ Comment at: clangd/ExpectedTypes.cpp:40 + +llvm::Optional<std::string> encodeType(ASTContext &Ctx, QualType T) { + assert(!T.isNull()); ---------------- IIUC, we also encode the qualifiers into the final representation? If so, have you considered the underlying type without qualifiers? It seems to me this might be too restrictive for type-based boosting. For code completion ranking, I think type qualifiers (`const` etc) can be separate signals. ================ Comment at: clangd/ExpectedTypes.h:10 +// A simplified model of C++ types that can be used to check whether they are +// convertible between each other without looking at the ASTs. +// Used for code completion ranking. ---------------- We might want to formalize what "convertible" means here. E.g. does it cover conversion between base and derived class? Does it cover double <-> int conversion? ================ Comment at: clangd/ExpectedTypes.h:29 +namespace clangd { +/// An opaque representation of a type that can be computed based on clang AST +/// and compared for equality. The encoding is stable between different ASTs, ---------------- The name seems opaque ;) Why is it `opaque`? ================ Comment at: clangd/ExpectedTypes.h:37 + fromCompletionResult(ASTContext &Ctx, const CodeCompletionResult &R); + static llvm::Optional<OpaqueType> fromPreferredType(ASTContext &Ctx, + QualType Type); ---------------- why "preferred type"? maybe add a comment? ================ Comment at: clangd/ExpectedTypes.h:40 + + /// Get the raw byte representation of the type. Bitwise equality of the raw + /// data is equivalent to equality operators of SType itself. The raw ---------------- What is the raw representation? A hash or the type name or USR? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52273 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits