[PATCH] D19816: [find-all-symbols] Add IWYU private pragma support.

[Manuel Klimek via cfe-commits]

klimek added inline comments.

================
Comment at: include-fixer/find-all-symbols/FindAllSymbols.cpp:173-182
@@ -165,1 +172,12 @@
 
+void FindAllSymbols::addPragmaHeader(FileID ID, llvm::StringRef FilePath) {
+  PragmaHeaderMap[ID] = FilePath;
+}
+
+llvm::Optional<std::string> FindAllSymbols::getPragmaHeader(FileID ID) const {
+  auto It = PragmaHeaderMap.find(ID);
+  if (It == PragmaHeaderMap.end())
+    return llvm::None;
+  return It->second;
+}
+
----------------
It seems weird to add this and just forward the interface.

================
Comment at: include-fixer/find-all-symbols/FindAllSymbols.h:49
@@ -45,1 +48,3 @@
 
+  void addPragmaHeader(FileID ID, llvm::StringRef FilePath);
+
----------------
I think the fact that those are generated via IWYU comments are an implementation detail, and this code doesn't care. Perhaps call it HeaderMapping or HeaderRemapping. Also, it's unclear what the semantics are, so I think this needs a comment.

================
Comment at: include-fixer/find-all-symbols/PragmaCommentHandler.h:23
@@ +22,3 @@
+public:
+  PragmaCommentHandler(FindAllSymbols *Matcher) : Matcher(Matcher) {}
+
----------------
I'd pull out an interface like HeaderMapCollector or ForwardingHeaderCollector, or even just pass in a std::function that collects the header. Or, just make this PragmaCommentHandler have a method to return a list of forwarding headers?
Generally, I think this couples the two classes much more than necessary.


Repository:
  rL LLVM

http://reviews.llvm.org/D19816