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