[PATCH] [clang-tidy] Add a generic header guard checker + LLVM implementation.

[Alexander Kornienko]

On Wed, Aug 13, 2014 at 4:01 PM, Benjamin Kramer <benny.kra at gmail.com>
wrote:

> ================
> Comment at: clang-tidy/llvm/HeaderGuardCheck.cpp:21
> @@ +20,3 @@
> +                                                 StringRef OldGuard) {
> +  std::string Guard = tooling::getAbsolutePath(Filename);
> +
> ----------------
> Alexander Kornienko wrote:
> > Can we use the location of the compilation database to make
> project-relative paths? We can easily pass compilation database to the
> check via ClangTidyContext, if need be. We should consider this, but it
> shouldn't block this patch, though.
> That's probably the right thing to do in the long term. Using absolute
> paths is a hack and might break if the user has a weird setup. We will get
> FileEntries with absolute paths from time to time (e.g. #include "./foo.h"
> creates them), so canonicalizing them with the compilation database seems
> like a nice way around this problem. It would break using without a
> database around (header files are often missing in the database), not sure
> what to do about that.
>

The simplest thing we can do, is use the path of the compilation database
as the project root, and make all file paths relative to it. The headers
don't have to be in the compilation database for this to work. This method
can also break in weird setups, but we should give this a try.


>
> ================
> Comment at: clang-tidy/llvm/HeaderGuardCheck.h:21
> @@ +20,3 @@
> +public:
> +  bool shouldSuggestEndifComment(StringRef Filename) override { return
> false; }
> +  bool shouldFixHeaderGuard(StringRef Filename) override;
> ----------------
> Alexander Kornienko wrote:
> > Why false? Aren't #endif comments used in LLVM?
> Some files do, the majority of headers does not, so I disabled them for
> now.
>
> ================
> Comment at: clang-tidy/llvm/HeaderGuardCheck.h:24
> @@ +23,3 @@
> +  std::string getHeaderGuard(StringRef Filename,
> +                             StringRef OldGuard = StringRef()) override;
> +};
> ----------------
> Alexander Kornienko wrote:
> > Do you need to provide a default argument in the overridden method? Are
> you using this specific method with one argument?
> I am calling it, but it should only be there in the base class. This was
> copy and paste failing.
>
> http://reviews.llvm.org/D4867
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140813/fa18d425/attachment.html>