<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Aug 13, 2014 at 4:01 PM, Benjamin Kramer <span dir="ltr"><<a href="mailto:benny.kra@gmail.com" target="_blank">benny.kra@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">================<br>
Comment at: clang-tidy/llvm/HeaderGuardCheck.cpp:21<br>
@@ +20,3 @@<br>
+                                                 StringRef OldGuard) {<br>
+  std::string Guard = tooling::getAbsolutePath(Filename);<br>
+<br>
----------------<br>
</div><div class="">Alexander Kornienko wrote:<br>
> 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.<br>

</div>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.<br>
</blockquote><div><br></div><div>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. </div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class=""><br>
================<br>
Comment at: clang-tidy/llvm/HeaderGuardCheck.h:21<br>
@@ +20,3 @@<br>
+public:<br>
+  bool shouldSuggestEndifComment(StringRef Filename) override { return false; }<br>
+  bool shouldFixHeaderGuard(StringRef Filename) override;<br>
----------------<br>
</div><div class="">Alexander Kornienko wrote:<br>
> Why false? Aren't #endif comments used in LLVM?<br>
</div>Some files do, the majority of headers does not, so I disabled them for now.<br>
<div class=""><br>
================<br>
Comment at: clang-tidy/llvm/HeaderGuardCheck.h:24<br>
@@ +23,3 @@<br>
+  std::string getHeaderGuard(StringRef Filename,<br>
+                             StringRef OldGuard = StringRef()) override;<br>
+};<br>
----------------<br>
</div><div class="">Alexander Kornienko wrote:<br>
> Do you need to provide a default argument in the overridden method? Are you using this specific method with one argument?<br>
</div>I am calling it, but it should only be there in the base class. This was copy and paste failing.<br>
<br>
<a href="http://reviews.llvm.org/D4867" target="_blank">http://reviews.llvm.org/D4867</a><br>
<br></blockquote></div>
</div></div>