[PATCH] D134235: [clang-doc] Clean up *Info constructors.
Haowei Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 20 16:39:53 PDT 2022
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
More information about the cfe-commits
mailing list