[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