kbobyrev added inline comments.
================ Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:64 *Serialized->mutable_location()->mutable_file_path() = std::string(); Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized); EXPECT_FALSE(Deserialized); ---------------- kadircet wrote: > nit: can be directly asserted > > Do you mean "directly checked (whether via `EXPECTED` or `ASSERT`, I guess `EXPECTED` should be still better here, right?)" here and elsewhere? If so, done! ================ Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:159 Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized); EXPECT_TRUE(Deserialized); ---------------- kadircet wrote: > maybe make this an assert too (and also consider issuing ASSERT_FALSE > directly on the expression instead of an extra assingment) Why is would asserting be better here? If I'm not dereferencing it I guess this should be worse because only the first assertion failure is checked, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84535/new/ https://reviews.llvm.org/D84535 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
