This is an archive of the discontinued LLVM Phabricator instance.

[clang-doc] Serialize child namespaces and records
ClosedPublic

Authored by DiegoAstiazaran on Jun 27 2019, 5:26 PM.

Details

Summary

Serialization of child namespaces and records is now handled.
Namespaces can have child records and child namespaces.
Records can only have child records.

Diff Detail

Event Timeline

juliehockett added inline comments.Jun 27 2019, 5:45 PM
clang-tools-extra/clang-doc/Serialize.cpp
341

You're probably going to want the path in this too, here and below

402–403

Maybe return the parent in the second position, and then comment about how the info itself is wrapped in its parent scope? That would make more sense logically, and they all end up in the same place. Same for methods/enums.

clang-tools-extra/clang-doc/Serialize.h
30–53

Comment here what each of these represents, e.g. the first info contains the relevant information about the declaration, the second records the parent.

clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
41–45

Extract this logic into a mapDecl function, since it's repeated so much

334

Just use the EmptySID -- the checking function ignores the USRs since they can be system-dependent. Here and below.

DiegoAstiazaran marked 6 inline comments as done.

Add comments.
Extract repeated logic into mapDecl function in SerializeTest.cpp
Use EmptySID instead of specific USR in tests.
Use {} instead of {nullptr, nullptr}.

clang-tools-extra/clang-doc/Serialize.cpp
341

This will be added when D63663 is pushed.

juliehockett added inline comments.Jun 28 2019, 1:03 PM
clang-tools-extra/clang-doc/Serialize.cpp
366

Can you make this a switch statement, doing the appropriate things for IT_record and IT_namespace and using llvm_unreachable("Invalid reference type") for all other possible values? This will help prevent silent regressions down the line.

374

nit: generally try to make comments full sentences

402–403

nit: spelling, here and below

DiegoAstiazaran marked 3 inline comments as done.

Change an if statement for a switch statement in emitInfo(const RecordDecl *D, ...), it now handles correctly unexpected info types.
Fix spelling in comments.

juliehockett accepted this revision.Jul 2 2019, 10:47 AM
This revision is now accepted and ready to land.Jul 2 2019, 10:47 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 2 2019, 1:00 PM