[clang-format] [NFC] Ensure clang-format is itself clang-formatted.

[MyDeveloperDay]

Summary

Before making a proposed change, ensure ClangFormat.cpp is fully clang-formatted,

no functional change just clang-formatting using the in tree .clang-format.

[Eugene.Zelenko]

If clang-format if clang-formatted now completely, I think will be good idea to add rule to check this during build, like Polly does.

[MyDeveloperDay]

In D68551#1696598, @Eugene.Zelenko wrote:

If clang-format if clang-formatted now completely, I think will be good idea to add rule to check this during build, like Polly does.

This is a great idea... Do you know if is this run as part of the build or as a custom rule they have to remember to run? I guess some additional checking might be needed to allow bootstrapping this, but I really like this idea of once a directory is clean keeping it clean because of its part of a normal build.

Keeping code clang-format clean with all the best will in the world is super hard, changes drift in very slowly even with asking people to git-clang-format then if one is missing you are asking for what I'm doing here which is making a clang-format only commit and some don't like that.

# Add target to check formatting of polly files
file( GLOB_RECURSE files *.h lib/*.cpp lib/*.c tools/*.cpp tools/*.c tools/*.h unittests/*.cpp)
file( GLOB_RECURSE external lib/External/*.h lib/External/*.c lib/External/*.cpp isl_config.h)
list( REMOVE_ITEM files ${external})

set(check_format_depends)
set(update_format_depends)
set(i 0)
foreach (file IN LISTS files)
  add_custom_command(OUTPUT polly-check-format${i}
    COMMAND clang-format -sort-includes -style=llvm ${file} | diff -u ${file} -
    VERBATIM
    COMMENT "Checking format of ${file}..."
  )
  list(APPEND check_format_depends "polly-check-format${i}")

  add_custom_command(OUTPUT polly-update-format${i}
    COMMAND clang-format -sort-includes -i -style=llvm ${file}
    VERBATIM
    COMMENT "Updating format of ${file}..."
  )
  list(APPEND update_format_depends "polly-update-format${i}")

  math(EXPR i ${i}+1)
endforeach ()

add_custom_target(polly-check-format DEPENDS ${check_format_depends})
set_target_properties(polly-check-format PROPERTIES FOLDER "Polly")

add_custom_target(polly-update-format DEPENDS ${update_format_depends})
set_target_properties(polly-update-format PROPERTIES FOLDER "Polly")

[MyDeveloperDay]

I think when polly's script detects a change it's just showing the diff, this might be so much nicer if we could implement D68554: [clang-format] Proposal for clang-format to give compiler style warnings

[mitchell-stellar]

LGTM.

I agree that a system in place that either enforces clang-formatting on commit or after the fact would be ideal. Otherwise, I don't see a need to have to approve these NFC commits.

[Meinersbur]

In D68551#1697842, @mitchell-stellar wrote:

I agree that a system in place that either enforces clang-formatting on commit or after the fact would be ideal. Otherwise, I don't see a need to have to approve these NFC commits.

The current coding policy contains "Our long term goal is for the entire codebase to follow the convention, but we explicitly do not want patches that do large-scale reformatting of existing code." that was added after someone removed all trailing whitespace all LLVM files. Reformatting the code you are going to work in is fine, but not on the entire code base. Ideally we'd also run the regression tests in a pre-commit hook.

Btw, I am the author of the CMakeLists snippet quoted by @MyDeveloperDay. Before that, it was a shell script that didn't run on Windows. Making it part of the regression test basically eliminated all discussion about code formatting, but we had to run large-scale reformatting whenever clang-format changed in some way. It also runs by the polly-* buildbots which I personally do not like since I don't see code formatting as a reason why a build should fail.

[MyDeveloperDay]

Btw, I am the author of the CMakeLists snippet quoted by @MyDeveloperDay. Before that, it was a shell script that didn't run on Windows. Making it part of the regression test basically eliminated all discussion about code formatting, but we had to run large-scale reformatting whenever clang-format changed in some way. It also runs by the polly-* buildbots which I personally do not like since I don't see code formatting as a reason why a build should fail.

Thank you for your comment, do we have CMake infrastructure (I'm not a CMake expert) to be able to parameterize that snippet and put it somewhere centrally so that others could simply inherit this in their CMakeList.txt like:

file( GLOB files ../lib/Format/*.h ../lib/Format/*.cpp ../unittests/*.cpp ../include/clang/Format/*.h)
add_clang_format_target(XXX,files)

so that they'd get your XXX-check-format and XXX-update-format rules? It might help the proliferation of clang-formatted areas? (and keep them clean)

[Meinersbur]

We should seek having only one formatting policy for the entire project, it's too confusing otherwise (e.g. LLVMSupport/ADT is also used by clang-format, are they force-formatted as well? What about the tools/utils subdirectories?). If you would like to gradually move into that direction, it should be discussed on llvm-dev.

cfe/trunk/tools/clang-format/ClangFormat.cpp

Lengths("length",
                      "Multiple ranges can be formatted by specifying\n"
                      "several -offset and -length pairs.\n"
                      "When only a single -offset is specified without\n"
                      "-length, clang-format will format up to the end\n"
                      "of the file.\n"
                      "Can only be used with one input file."),
             cl::cat(ClangFormatCategory));
 static cl::list<std::string>
-LineRanges("lines", cl::desc("<start line>:<end line> - format a range of\n"
-                             "lines (both 1-based).\n"
-                             "Multiple ranges can be formatted by specifying\n"
-                             "several -lines arguments.\n"
-                             "Can't be used with -offset and -length.\n"
-                             "Can only be used with one input file."),
+    LineRanges("lines",
+               cl::desc("<start line>:<end line> - format a range of\n"
                cl::cat(ClangFormatCategory));
 static cl::opt<std::string>
     Style("style", cl::desc(clang::format::StyleOptionHelpDescription),
           cl::init(clang::format::DefaultFormatStyle),
           cl::cat(ClangFormatCategory));
 static cl::opt<std::string>
     FallbackStyle("fallback-style",
                   cl::desc("The name of the predefined style used as a\n"
                            "fallback in case clang-format is invoked with\n"
                            "-style=file, but can not find the .clang-format\n"
                            "file to use.\n"
                            "Use -fallback-style=none to skip formatting."),
                   cl::init(clang::format::DefaultFallbackStyle),
                   cl::cat(ClangFormatCategory));
 
-static cl::opt<std::string>
-AssumeFileName("assume-filename",
-               cl::desc("When reading from stdin, clang-format assumes this\n"
-                        "filename to look for a style config file (with\n"
-                        "-style=file) and to determine the language."),
-               cl::init("<stdin>"), cl::cat(ClangFormatCategory));
+static cl::opt<std::string> AssumeFileName(
+    "assume-filename",
 
 static cl::opt<bool> Inplace("i",
                              cl::desc("Inplace edit <file>s, if specified."),
                              cl::cat(ClangFormatCategory));
 
 static cl::opt<bool> OutputXML("output-replacements-xml",
                                cl::desc("Output replacements as XML."),
                                cl::cat(ClangFormatCategory));

 static bool format(StringRef FileName) {
   if (!OutputXML && Inplace && FileName == "-") {
     errs() << "error: cannot use -i when reading from stdin.\n";
     return false;
   }
   // On Windows, overwriting a file with an open file mapping doesn't work,
   // so read the whole file into memory when formatting in-place.
   ErrorOr<std::unique_ptr<MemoryBuffer>> CodeOrErr =
-      !OutputXML && Inplace ? MemoryBuffer::getFileAsStream(FileName) :
-                              MemoryBuffer::getFileOrSTDIN(FileName);
+      !OutputXML && Inplace ? MemoryBuffer::getFileAsStream(FileName)
+                            : MemoryBuffer::getFileOrSTDIN(FileName);
   if (std::error_code EC = CodeOrErr.getError()) {
     errs() << EC.message() << "\n";
     return true;
   }
   std::unique_ptr<llvm::MemoryBuffer> Code = std::move(CodeOrErr.get());
   if (Code->getBufferSize() == 0)
     return false; // Empty files are formatted correctly.
 
   // Check to see if the buffer has a UTF Byte Order Mark (BOM).
   // We only support UTF-8 with and without a BOM right now.  See
   // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
   // for more information.
   StringRef BufStr = Code->getBuffer();
-  const char *InvalidBOM = llvm::StringSwitch<const char *>(BufStr)
+  const char *InvalidBOM =
+      llvm::StringSwitch<const char *>(BufStr)
           .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
-                                                  "UTF-32 (BE)")
           .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
-                                                  "UTF-32 (LE)")
           .StartsWith("\xFE\xFF", "UTF-16 (BE)")
           .StartsWith("\xFF\xFE", "UTF-16 (LE)")
           .StartsWith("\x2B\x2F\x76", "UTF-7")
           .StartsWith("\xF7\x64\x4C", "UTF-1")
           .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
           .StartsWith("\x0E\xFE\xFF", "SCSU")
           .StartsWith("\xFB\xEE\x28", "BOCU-1")
           .StartsWith("\x84\x31\x95\x33", "GB-18030")
           .Default(nullptr);
 
   if (InvalidBOM) {
     errs() << "error: encoding with unsupported byte order mark \""
            << InvalidBOM << "\" detected";
     if (FileName != "-")
       errs() << " in file '" << FileName << "'";
     errs() << ".\n";
     return true;
static bool format(StringRef FileName) {
   auto ChangedCode = tooling::applyAllReplacements(Code->getBuffer(), Replaces);
   if (!ChangedCode) {
     llvm::errs() << llvm::toString(ChangedCode.takeError()) << "\n";
     return true;
   }
   // Get new affected ranges after sorting `#includes`.
   Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
   FormattingAttemptStatus Status;
-  Replacements FormatChanges = reformat(*FormatStyle, *ChangedCode, Ranges,
-                                        AssumedFileName, &Status);
+  Replacements FormatChanges =
+      reformat(*FormatStyle, *ChangedCode, Ranges, AssumedFileName, &Status);
   Replaces = Replaces.merge(FormatChanges);
   if (OutputXML) {
     outs() << "<?xml version='1.0'?>\n<replacements "
               "xml:space='preserve' incomplete_format='"
            << (Status.FormatComplete ? "false" : "true") << "'";
     if (!Status.FormatComplete)
       outs() << " line='" << Status.Line << "'";
     outs() << ">\n";
if (Inplace) {
         outs() << " }\n";
       }
       Rewrite.getEditBuffer(ID).write(outs());
     }
   }
   return false;
 }
 
-}  // namespace format
-}  // namespace clang
+} // namespace format
+} // namespace clang
 
 static void PrintVersion(raw_ostream &OS) {
   OS << clang::getClangToolFullVersion("clang-format") << '\n';
 }
 
 int main(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
 
if (DumpConfig) {
     return 0;
   }
 
   bool Error = false;
   if (FileNames.empty()) {
     Error = clang::format::format("-");
     return Error ? 1 : 0;
   }
-  if (FileNames.size() != 1 && (!Offsets.empty() || !Lengths.empty() || !LineRanges.empty())) {
+  if (FileNames.size() != 1 &&
+      (!Offsets.empty() || !Lengths.empty() || !LineRanges.empty())) {
     errs() << "error: -offset, -length and -lines can only be used for "
               "single file.\n";
     return 1;
   }
   for (const auto &FileName : FileNames) {
     if (Verbose)
       errs() << "Formatting " << FileName << "\n";
     Error |= clang::format::format(FileName);
   }
   return Error ? 1 : 0;
 }