[PATCH] D63584: [clang][AST] Refactoring ASTNameGenerator to use pimpl pattern (NFC).

Saleem Abdulrasool via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 19 18:51:49 PDT 2019


compnerd accepted this revision.
compnerd added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/AST/Mangle.cpp:343
 
-std::vector<std::string>
-ASTNameGenerator::getAllManglings(const ObjCContainerDecl *OCD) {
+  std::vector<std::string> getAllManglings(const ObjCContainerDecl *OCD) {
     StringRef ClassName;
----------------
plotfi wrote:
> @aaron.ballman I can move this down to the private section in a subsequent NFC if you'd like. 
The implementation is never leaked to the user.  This means that this is effectively private.


================
Comment at: clang/lib/AST/Mangle.cpp:416
 
-bool ASTNameGenerator::writeFuncOrVarName(const NamedDecl *D, raw_ostream &OS) {
+private:
+  bool writeFuncOrVarName(const NamedDecl *D, raw_ostream &OS) {
----------------
Don't think that it really matters to make this private or public really.  The implementation is fully private.


================
Comment at: clang/lib/AST/Mangle.cpp:473
+ASTNameGenerator::ASTNameGenerator(ASTContext &Ctx)
+    : Impl(new Implementation(Ctx)) {}
+
----------------
`llvm::make_unique` would be nicer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63584/new/

https://reviews.llvm.org/D63584





More information about the cfe-commits mailing list