juliehockett added inline comments.
================
Comment at: clang-doc/YAMLGenerator.cpp:265
+// and thus register the generator.
+volatile int YAMLGeneratorAnchorSource = 0;
+
----------------
ioeric wrote:
> nit: add `static`?
`static` declares it as file-local, and so the `extern` reference to this in
Generators.cpp wouldn't be allowed.
================
Comment at: clang-doc/generators/YAMLGenerator.cpp:223
+
+template <> struct MappingTraits<std::unique_ptr<CommentInfo>> {
+ static void mapping(IO &IO, std::unique_ptr<CommentInfo> &I) {
----------------
ioeric wrote:
> juliehockett wrote:
> > ioeric wrote:
> > > YAML mapping for `unique_ptr` is a bit unusual. I wonder whether this
> > > would work all the time e.g. if the unique_ptr has not been allocated.
> > Mmm yes it's a little weird -- that said, is there a better way emit the
> > info given a pointer?
> Not sure what the initial intention was for using `unique_ptr` here, but one
> option is to get rid of the `unique_ptr` and just store `CommentInfo` in the
> structure. Another (less ideal) option is to check whether `I` is nullptr and
> allocate when it's null (only when deserialization though; I think you could
> tell this from `IO`); and you'd probably want to avoid the allocation when
> the comment info is actually not present in `IO` (IIRC, mapping would be
> called even when the info is not present).
Sorry this slipped through before -- it's a pointer because it has to be able
to store itself (the other infos just store the `CommentInfo` directly, but
each `CommentInfo` can have comment children). The pointer only appears there.
================
Comment at: clang-doc/tool/ClangDocMain.cpp:221
+ }
+ std::error_code FileErr;
+ llvm::raw_fd_ostream InfoOS(InfoPath.get(), FileErr,
llvm::sys::fs::F_None);
----------------
ioeric wrote:
> nit: I wonder if you could merge creation of `InfoOS` into `getPath` so that
> the helper function would just return `llvm::Expected<raw_fd_ostream>`
You can't, because when you return `llvm::make_error` it invokes a copy
constructor, which is deleted in `raw_ostream`s.
https://reviews.llvm.org/D43667
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
