Eric Liu <ioe...@google.com> schrieb am Di., 25. Sep. 2018, 01:22: > On Tue, Sep 25, 2018 at 6:34 AM Ilya Biryukov <ibiryu...@google.com> > wrote: > >> Why would we want to serialize the origin? >> > We only serialize and deserialize for the static index, it does not seem >> to be useful to serialize origin in that scenario. >> > We serialize Origin because it's a property of Symbol? The origin for the > current YAML symbols is defaulted to "unknown". > My view would be that it's a property of a symbol living in memory, but not the serialized one. E.g. all symbols read from the yaml index should have the static origin. If we store an origin, we allow loading symbols with non-static origin, it's hard to see how that would be useful.
> It's true that we *currently* only serialize symbols from static index, > but there is nothing preventing us from doing it for other indexes with > different origins. You could probably override origins when loading static > index, but that doesn't seem to work in general. > The origin seems to be exactly the field that is set and manipulated by index implementations, but they all have the knowledge on what their origin is or how to combine origins of subindexes. Again, it seems there are two classes hiding in symbol. All other fields provide useful information about C++ semantics of the symbol, while origin provides some traceability when combining indexes. The former is something we need to serialize, the latter is something we can infer when deserializing. If we will have a use case for serializing the latter entity(with origin) for any reason, we might add that separately. Not sure if it's worth the trouble changing it, just wanted to point out that storing the origin for each symbol in the yaml index for the purpose of indicating that the symbol is in the yaml index seems to be a bit off. > I checked this in without review as I thought this was a trivial fix. The > binary serialization also serializes the Origin field. > LG to be consistent with binary serialization, the same question applies to binary serialization. Again, the change itself seems fine, just nitpicking on whether putting origin into a symbol at that level makes sense. >> Am I missing something? >> > >> On Fri, Sep 21, 2018 at 3:06 PM Eric Liu via cfe-commits < >> cfe-commits@lists.llvm.org> wrote: >> >>> Author: ioeric >>> Date: Fri Sep 21 06:04:57 2018 >>> New Revision: 342730 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=342730&view=rev >>> Log: >>> [clangd] Remember to serialize symbol origin in YAML. >>> >>> Modified: >>> clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp >>> clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp >>> >>> Modified: clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp?rev=342730&r1=342729&r2=342730&view=diff >>> >>> ============================================================================== >>> --- clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp (original) >>> +++ clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp Fri Sep 21 >>> 06:04:57 2018 >>> @@ -27,6 +27,7 @@ namespace yaml { >>> >>> using clang::clangd::Symbol; >>> using clang::clangd::SymbolID; >>> +using clang::clangd::SymbolOrigin; >>> using clang::clangd::SymbolLocation; >>> using clang::index::SymbolInfo; >>> using clang::index::SymbolKind; >>> @@ -65,6 +66,17 @@ struct NormalizedSymbolFlag { >>> uint8_t Flag = 0; >>> }; >>> >>> +struct NormalizedSymbolOrigin { >>> + NormalizedSymbolOrigin(IO &) {} >>> + NormalizedSymbolOrigin(IO &, SymbolOrigin O) { >>> + Origin = static_cast<uint8_t>(O); >>> + } >>> + >>> + SymbolOrigin denormalize(IO &) { return >>> static_cast<SymbolOrigin>(Origin); } >>> + >>> + uint8_t Origin = 0; >>> +}; >>> + >>> template <> struct MappingTraits<SymbolLocation::Position> { >>> static void mapping(IO &IO, SymbolLocation::Position &Value) { >>> IO.mapRequired("Line", Value.Line); >>> @@ -102,6 +114,8 @@ template <> struct MappingTraits<Symbol> >>> MappingNormalization<NormalizedSymbolID, SymbolID> NSymbolID(IO, >>> Sym.ID); >>> MappingNormalization<NormalizedSymbolFlag, Symbol::SymbolFlag> >>> NSymbolFlag( >>> IO, Sym.Flags); >>> + MappingNormalization<NormalizedSymbolOrigin, SymbolOrigin> >>> NSymbolOrigin( >>> + IO, Sym.Origin); >>> IO.mapRequired("ID", NSymbolID->HexString); >>> IO.mapRequired("Name", Sym.Name); >>> IO.mapRequired("Scope", Sym.Scope); >>> @@ -110,6 +124,7 @@ template <> struct MappingTraits<Symbol> >>> SymbolLocation()); >>> IO.mapOptional("Definition", Sym.Definition, SymbolLocation()); >>> IO.mapOptional("References", Sym.References, 0u); >>> + IO.mapOptional("Origin", NSymbolOrigin->Origin); >>> IO.mapOptional("Flags", NSymbolFlag->Flag); >>> IO.mapOptional("Signature", Sym.Signature); >>> IO.mapOptional("CompletionSnippetSuffix", >>> Sym.CompletionSnippetSuffix); >>> >>> Modified: clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp?rev=342730&r1=342729&r2=342730&view=diff >>> >>> ============================================================================== >>> --- clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp >>> (original) >>> +++ clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp Fri >>> Sep 21 06:04:57 2018 >>> @@ -7,6 +7,7 @@ >>> // >>> >>> >>> //===----------------------------------------------------------------------===// >>> >>> +#include "index/Index.h" >>> #include "index/Serialization.h" >>> #include "index/SymbolYAML.h" >>> #include "llvm/Support/ScopedPrinter.h" >>> @@ -35,6 +36,7 @@ CanonicalDeclaration: >>> End: >>> Line: 1 >>> Column: 1 >>> +Origin: 4 >>> Flags: 1 >>> Documentation: 'Foo doc' >>> ReturnType: 'int' >>> @@ -82,6 +84,7 @@ TEST(SerializationTest, YAMLConversions) >>> EXPECT_EQ(Sym1.Documentation, "Foo doc"); >>> EXPECT_EQ(Sym1.ReturnType, "int"); >>> EXPECT_EQ(Sym1.CanonicalDeclaration.FileURI, "file:///path/foo.h"); >>> + EXPECT_EQ(Sym1.Origin, SymbolOrigin::Static); >>> EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion); >>> EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated); >>> EXPECT_THAT(Sym1.IncludeHeaders, >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >> >> >> -- >> Regards, >> Ilya Biryukov >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
