kadircet added inline comments.
================ Comment at: clang-tools-extra/clangd/Hover.cpp:571 + if (auto Size = Ctx.getTypeSizeInCharsIfKnown(RD->getTypeForDecl())) + HI.Size = Size->getQuantity(); + ---------------- QuantityType seems to be an alias for int64_t, not sure how likely it is that someone will have a type that's bigger than 4GB, but still, should we make typeof HoverInfo::Size optional<int64_t> instead of unsigned? ================ Comment at: clang-tools-extra/clangd/Hover.cpp:573 + + if (const auto *FD = llvm::dyn_cast<FieldDecl>(&ND)) { + const auto *Record = FD->getParent()->getDefinition(); ---------------- nit: either make this `else if` or put a return above ? ================ Comment at: clang-tools-extra/clangd/Hover.cpp:576 + if (Record && !Record->isDependentType()) { + unsigned OffsetBits = Ctx.getFieldOffset(FD); + if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType())) { ---------------- similar to size, offset is also of type uint64_t ================ Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:69 struct Foo { - int [[b^ar]]; + char [[b^ar]]; }; ---------------- any reason for changing these from int to char ? ================ Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:1830 HI.Kind = index::SymbolKind::Class; + HI.Size = 10; HI.TemplateParameters = { ---------------- maybe also add offset? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77355/new/ https://reviews.llvm.org/D77355 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits