[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 23 07:44:31 PST 2017


sammccall added a comment.

Thanks for bearing with me here :)



================
Comment at: include-fixer/InMemorySymbolIndex.h:27
 
-  std::vector<clang::find_all_symbols::SymbolInfo>
+  std::vector<clang::find_all_symbols::SymbolAndSignals>
   search(llvm::StringRef Identifier) override;
----------------
hokein wrote:
> sammccall wrote:
> > hokein wrote:
> > > There are many places using `std::vector<clang::find_all_symbols::SymbolAndSignals>`. Maybe we can use a type alias for it, so that we can type less.  
> > I guess? It's the namespaces that are the problem (vector<SymbolAndSignals> is fine) and most of the namespace noise wouldn't go away here.
> > 
> > is `clang::find_all_symbols::SymbolsSignalsList` better enough to obscure what the actual type is? It's 45 chars vs 54.
> > 
> > IMO it's not worth it here, though `clang::find_all_symbols::SymbolInfo::SignalMap` vs `std::map<clang::find_all_symbols::SymbolInfo, clang::find_all_symbols::SymbolInfo::Signals>` is.
> If we put the type alias under `clang::include_fixer` namespace, it will shorten the name more. Agree it is not worth the effect as the full name only happens in headers. 
> 
> We could save a few characters by getting rid of `clang` because we are always in `clang` namespace. So `std::vector<find_all_symbols::SymbolAndSignals>` should work, this looks slightly better. :)
Done, also cleaned up other redundant namespaces in touched files.


https://reviews.llvm.org/D30210





More information about the cfe-commits mailing list