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

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 12 18:13:15 PST 2016


hokein added inline comments.

================
Comment at: clang-tidy/ClangTidyOptions.h:216
@@ +215,3 @@
+/// HeaderFileExtensions.
+bool endWithHeaderFileExtensions(llvm::StringRef FileName,
+                                 llvm::StringRef HeaderFileExtensions);
----------------
aaron.ballman wrote:
> endsWithHeaderFileExtension instead? However, given that uses of this all start with a SourceLocation, I wonder if that makes for a cleaner API: isLocInHeaderFile(SourceLocation, <Extensions>);
> 
> Also, how does this work if I want to include an extension-less file as the header file "extension?" It would be plausible if the extensions were passed in as a list, but as it stands it doesn't seem possible without weird conventions like leaving a blank in the list (e.g., `.h,,.hpp`), which seems error-prone.
> 
> Also, I'm not certain what I can pass in. The documentation should be updated to state whether these extensions are intended to include the ".".
> endsWithHeaderFileExtension instead? However, given that uses of this all start with a SourceLocation, I wonder if that makes for a cleaner API: isLocInHeaderFile(SourceLocation, <Extensions>);

Using `SourceLocation` only is not enough to retrieve the belonging file name (we need `SourceManager` too).

>Also, how does this work if I want to include an extension-less file as the header file "extension?" It would be plausible if the extensions were passed in as a list, but as it stands it doesn't seem possible without weird conventions like leaving a blank in the list (e.g., .h,,.hpp), which seems error-prone.

Yeah, for extensions-less header file, you can pass the string like `.h,,.hpp`, which is a bit of weird. Do you have a better idea here? Passing a string into `header-file-extensions` seems the most reasonable choice. 



Repository:
  rL LLVM

http://reviews.llvm.org/D16113





More information about the cfe-commits mailing list