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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
