[PATCH] D83223: [clang-tidy] Header guard check can skip past license comment

[Nathan James via Phabricator via cfe-commits]

njames93 marked 2 inline comments as done.
njames93 added a comment.

In D83223#2147072 <https://reviews.llvm.org/D83223#2147072>, @aaron.ballman wrote:

> >   // This is not identified as a license comment as the
> >   // block is followed by code.
> >   void foo();
>
> FWIW: https://github.com/GrammaTech/gtirb-pprinter/blob/master/include/gtirb_pprinter/AttPrettyPrinter.hpp or https://github.com/GrammaTech/gtirb/blob/master/include/gtirb/AuxData.hpp (so there are projects which do not put a newline between the license and code).


Short of creating an AI that understands context it won't be possible to determine the difference between license and general documentation, in any case I feel this heuristic is the safest way to ensure good coverage with minimised risk of inserting the guard in the middle of documentation,



================
Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:48
   explicit MissingOptionError(std::string OptionName)
-      : OptionName(OptionName) {}
+      : OptionName(std::move(OptionName)) {}
 
----------------
aaron.ballman wrote:
> It makes me sad how much we're having to sprinkle `std::move()` calls around (I find the calls to `move()` to be more distracting that declaring the parameter types to be `const std::string&`.) Were these found mechanically?
These are me screwing around with my branches and will be removed


================
Comment at: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp:224
 }
+
+TEST(LLVMHeaderGuardCheckTest, FixHeaderGuardsWithComments) {
----------------
aaron.ballman wrote:
> You seem to be missing tests that show the license and header guards are both correct and found no issues.
That wouldn't be testing new behaviour added here. The check can already find header guards when there is a license, The code I have added only affects when no guard is found and it needs to add one.  I can still add those cases for piece of mind.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83223/new/

https://reviews.llvm.org/D83223