aaron.ballman added a comment. > // 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). ================ Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:48 explicit MissingOptionError(std::string OptionName) - : OptionName(OptionName) {} + : OptionName(std::move(OptionName)) {} ---------------- 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? ================ Comment at: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp:313 continue; - + StringRef Opening = AddNewLine == NewLineInsertions::None ? "#ifndef " + : AddNewLine == NewLineInsertions::One ---------------- njames93 wrote: > clang-format(trunk) says what I submitted was correct, clang-format-10 is > suggesting the change. Which do I trust?? Either is defensible, I'm fine with using what trunk says. ================ Comment at: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp:12 // FIXME: It seems this might be incompatible to dos path. Investigating. #if !defined(_WIN32) ---------------- Unrelated to this patch, but neat to know that there is zero testing of this on Windows. ================ Comment at: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp:224 } + +TEST(LLVMHeaderGuardCheckTest, FixHeaderGuardsWithComments) { ---------------- You seem to be missing tests that show the license and header guards are both correct and found no issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83223/new/ https://reviews.llvm.org/D83223 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits