[PATCH] D16113: [clang-tdiy] Add header file extension configuration support.

[Alexander Kornienko via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

Looks good with a couple of comments. Thanks for working on this!


================
Comment at: clang-tidy/utils/HeaderFileExtensionsUtils.cpp:45
@@ +44,3 @@
+
+bool parseHeaderFileExtensions(llvm::StringRef AllHeaderFileExtensions,
+                               HeaderFileExtensionsSet &HeaderFileExtensions,
----------------
No need for llvm:: here.

================
Comment at: clang-tidy/utils/HeaderFileExtensionsUtils.cpp:46
@@ +45,3 @@
+bool parseHeaderFileExtensions(llvm::StringRef AllHeaderFileExtensions,
+                               HeaderFileExtensionsSet &HeaderFileExtensions,
+                               char delimiter) {
----------------
Actually, this could be changed to a more generic `parseStringSetOption` or something similar. Let's do this in a follow up.

But for now we need to change the output parameter type to llvm::SmallSetImpl<StringRef> to avoid hardcoding the small size template argument. The typedef is then not useful anymore.

================
Comment at: clang-tidy/utils/HeaderFileExtensionsUtils.h:34
@@ +33,3 @@
+bool isPresumedLocInHeaderFile(
+    SourceLocation Loc, SourceManager &SM,
+    const HeaderFileExtensionsSet &HeaderFileExtensions);
----------------
hokein wrote:
> It is used in `UnnamedNamespaceInHeaderCheck`.
Ah, missed this somehow. Then it's fine. I'm not sure though, whether PresumedLocation is the right thing in that check, but we can figure this out later.


http://reviews.llvm.org/D16113