juliehockett added inline comments.
================
Comment at: clang-doc/Representation.h:138
+ SymbolID USR =
+ SymbolID(); // Unique identifier for the decl described by this Info.
+ const InfoType IT = InfoType::IT_default; // InfoType of this particular
Info.
----------------
ioeric wrote:
> Shouldn't USR be `SymbolID()` by default?
It actually is garbage by default -- doing this zeroed it out.
================
Comment at: clang-doc/Representation.h:146
+protected:
+ bool mergeBase(Info &&I);
};
----------------
ioeric wrote:
> It's a bit awkward that users have to dispatch from info types to the
> corresponding `merge` function (as in `Reducer.cpp`). I think it would make
> users' life easier if the library handles the dispatching.
>
> I wonder if something like the following would be better:
> ```
> struct Info {
> std::unique_ptr<Info> merge(const Indo& LHS, const Info& RHS);
> };
> // A standalone function.
> std::unique_ptr<Info> mergeInfo(const Info &LHS, const Info& RHS) {
> // merge base info.
> ...
> // merge subclass infos.
> assert(LHS.IT == RHS.IT); // or return nullptr
> switch (LHS.IT) {
> ...
> return Namespace::merge(LHS, RHS);
> }
> }
>
> struct NamespaceInfo : public Info {
> std::unique_ptr<Info> merge(LHS, RHS);
> };
>
> ```
>
> The unhandled switch case warning in compiler would help you catch
> unimplemented `merge` when new info types are added.
Sort of addressed in this update. There's an issue with where we allocate the
return pointer, because we need to know the type of info at allocation time --
let me know if what's here now is too far off of what you were suggesting.
================
Comment at: clang-doc/tool/ClangDocMain.cpp:181
+ doc::writeInfo(I.get(), Buffer);
+ if (DumpResultToFile("bc", Group.getKey() + ".bc", Buffer))
+ return 1;
----------------
ioeric wrote:
> (Sorry that I might be missing context here.)
>
> Could you please explain the incentive for dumping each info group to one bc
> file? If the key identifies a symbol (e.g. USR), wouldn't this result in a
> bitcode file being created for each symbol? This doesn't seem very scalable.
Yes, it would. This is mostly for debugging, since there's not really any tools
outside the clang system that would actually want/be able to use this
information.
https://reviews.llvm.org/D43341
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
