kimgr added a subscriber: kimgr.
kimgr added a comment.
Re semantics, you may want to link to IWYU's docs at
https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md
.
- Kim
Den 3 maj 2016 6:34 em skrev "Manuel Klimek via cfe-commits" <
cfe-commits@lists.llvm.org>:
> 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
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
