[PATCH] D20496: [include-fixer] Added find-stl-symbols to retrieve symbols (with the correct include header name) from C++ standard STL headers.

Benjamin Kramer via cfe-commits cfe-commits at lists.llvm.org
Mon May 23 07:29:53 PDT 2016


bkramer added inline comments.

================
Comment at: include-fixer/find-all-symbols/FindAllSymbols.h:47
@@ +46,3 @@
+protected:
+  // The way SymbolInfo of a decl is reported can be overrided.
+  virtual void reportDecl(const SourceManager &SM, const clang::NamedDecl *ND,
----------------
overridden

================
Comment at: include-fixer/find-all-symbols/SymbolReporter.h:25
@@ +24,3 @@
+  struct Args {
+    int Arg1;
+  };
----------------
Any reason not to call this IncludeDepth? Or make it a generic 'Priority' thing and throw away all the overrides? I'm not opposed to changing find-all-symbols to support that if it's less code.

================
Comment at: include-fixer/find-stl-symbols/FindSTLSymbolsAction.h:13
@@ +12,3 @@
+
+#include "../find-all-symbols/FindAllMacros.h"
+#include "../find-all-symbols/FindAllSymbols.h"
----------------
The .. in the include path is a strong indicator that we have weird layering. Would it make sense to merge find-stl-symbols into find-all-symbols?

================
Comment at: include-fixer/find-stl-symbols/STLSymbolsFinder.cpp:33
@@ +32,3 @@
+    : Files(Files), CXXIncludePath(CXXIncludePath), Verbose(Verbose) {
+  if (*CXXIncludePath.rbegin() != '/')
+    this->CXXIncludePath.push_back('/');
----------------
.back()

================
Comment at: include-fixer/find-stl-symbols/tool/FindSTLSymbolsMain.cpp:57
@@ +56,3 @@
+    }
+    llvm::raw_fd_ostream OS(FD, /*shouldClose*/ true);
+    WriteSymbolInfosToStream(OS, Symbols);
----------------
Just use the raw_fd_ostream constructor that takes a file name.


http://reviews.llvm.org/D20496





More information about the cfe-commits mailing list