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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to