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

[Kim Gräsman via cfe-commits]

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 at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160504/be1075f2/attachment.html>