haowei added a comment.
I am a bit puzzled by the intent of cleaning up the constructors in Info
object. I didn't see it as an issue to have multiple constructors, which is a
common practice. After this clean up It doesn't make it easier to use these
objects, in fact, in `Serialize.cpp`, you have to explicitly specify a new
SymbolID() when creating a new Reference object. While this change won't affect
(except for the global namespace) the output from clang-docs, it is not
semantically equivalent before the cleanup.
Would you mind explaining the reasons for removing these constructors?
================
Comment at: clang-tools-extra/clang-doc/Representation.h:165
- : Type(Type, Field, IT) {}
- TypeInfo(SymbolID Type, StringRef Field, InfoType IT, StringRef Path)
- : Type(Type, Field, IT, Path) {}
----------------
The clean up here prevents setting a customized SymbolID for TypeInfo, which
becomes a problem in the HTMLGeneratorTest, in which TypeInfo is constructed
with an constant EmptySID. While the outcome is the same, it is not
semantically equivalent.
================
Comment at: clang-tools-extra/clang-doc/Representation.h:219
- int LineNumber; // Line number of this Location.
+ int LineNumber = 0; // Line number of this Location.
SmallString<32> Filename; // File for this Location.
----------------
This is already set in the constructor. Does it still need to be set explicitly
here?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134235/new/
https://reviews.llvm.org/D134235
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
