[clang-format][NFC] Clean up the driver and getStyle() in Format.cpp

[owenca]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/74794.diff

2 Files Affected:

• (modified) clang/lib/Format/Format.cpp (+48-47)
• (modified) clang/tools/clang-format/ClangFormat.cpp (+17-23)

diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index b09487435adb2d..8feee7457fc31b 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3955,10 +3955,7 @@ llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName,
                                      StringRef FallbackStyleName,
                                      StringRef Code, llvm::vfs::FileSystem *FS,
                                      bool AllowUnknownOptions) {
-  if (!FS)
-    FS = llvm::vfs::getRealFileSystem().get();
   FormatStyle Style = getLLVMStyle(guessLanguage(FileName, Code));
-
   FormatStyle FallbackStyle = getNoStyle();
   if (!getPredefinedStyle(FallbackStyleName, Style.Language, &FallbackStyle))
     return make_string_error("Invalid fallback style: " + FallbackStyleName);
@@ -3974,14 +3971,18 @@ llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName,
                                AllowUnknownOptions)) {
       return make_string_error("Error parsing -style: " + ec.message());
     }
-    if (Style.InheritsParentConfig) {
-      ChildFormatTextToApply.emplace_back(
-          llvm::MemoryBuffer::getMemBuffer(StyleName, Source, false));
-    } else {
+
+    if (!Style.InheritsParentConfig)
       return Style;
-    }
+
+    ChildFormatTextToApply.emplace_back(
+        llvm::MemoryBuffer::getMemBuffer(StyleName, Source, false));
   }
 
+  if (!FS)
+    FS = llvm::vfs::getRealFileSystem().get();
+  assert(FS);
+
   // User provided clang-format file using -style=file:path/to/format/file.
   if (!Style.InheritsParentConfig &&
       StyleName.starts_with_insensitive("file:")) {
@@ -4015,18 +4016,12 @@ llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName,
       return Style;
   }
 
-  // Reset possible inheritance
-  Style.InheritsParentConfig = false;
-
-  // Look for .clang-format/_clang-format file in the file's parent directories.
-  SmallString<128> UnsuitableConfigFiles;
   SmallString<128> Path(FileName);
   if (std::error_code EC = FS->makeAbsolute(Path))
     return make_string_error(EC.message());
 
-  llvm::SmallVector<std::string, 2> FilesToLookFor;
-  FilesToLookFor.push_back(".clang-format");
-  FilesToLookFor.push_back("_clang-format");
+  // Reset possible inheritance
+  Style.InheritsParentConfig = false;
 
   auto dropDiagnosticHandler = [](const llvm::SMDiagnostic &, void *) {};
 
@@ -4040,9 +4035,14 @@ llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName,
     }
   };
 
+  // Look for .clang-format/_clang-format file in the file's parent directories.
+  llvm::SmallVector<std::string, 2> FilesToLookFor;
+  FilesToLookFor.push_back(".clang-format");
+  FilesToLookFor.push_back("_clang-format");
+
+  SmallString<128> UnsuitableConfigFiles;
   for (StringRef Directory = Path; !Directory.empty();
        Directory = llvm::sys::path::parent_path(Directory)) {
-
     auto Status = FS->status(Directory);
     if (!Status ||
         Status->getType() != llvm::sys::fs::file_type::directory_file) {
@@ -4055,50 +4055,51 @@ llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName,
       llvm::sys::path::append(ConfigFile, F);
       LLVM_DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n");
 
-      Status = FS->status(ConfigFile.str());
-
-      if (Status &&
-          (Status->getType() == llvm::sys::fs::file_type::regular_file)) {
-        llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text =
-            loadAndParseConfigFile(ConfigFile, FS, &Style, AllowUnknownOptions);
-        if (auto EC = Text.getError()) {
-          if (EC == ParseError::Unsuitable) {
-            if (!UnsuitableConfigFiles.empty())
-              UnsuitableConfigFiles.append(", ");
-            UnsuitableConfigFiles.append(ConfigFile);
-            continue;
-          }
+      Status = FS->status(ConfigFile);
+      if (!Status ||
+          Status->getType() != llvm::sys::fs::file_type::regular_file) {
+        continue;
+      }
+
+      llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text =
+          loadAndParseConfigFile(ConfigFile, FS, &Style, AllowUnknownOptions);
+      if (auto EC = Text.getError()) {
+        if (EC != ParseError::Unsuitable) {
           return make_string_error("Error reading " + ConfigFile + ": " +
                                    EC.message());
         }
-        LLVM_DEBUG(llvm::dbgs()
-                   << "Using configuration file " << ConfigFile << "\n");
+        if (!UnsuitableConfigFiles.empty())
+          UnsuitableConfigFiles.append(", ");
+        UnsuitableConfigFiles.append(ConfigFile);
+        continue;
+      }
 
-        if (!Style.InheritsParentConfig) {
-          if (ChildFormatTextToApply.empty())
-            return Style;
+      LLVM_DEBUG(llvm::dbgs()
+                 << "Using configuration file " << ConfigFile << "\n");
 
+      if (!Style.InheritsParentConfig) {
+        if (!ChildFormatTextToApply.empty()) {
           LLVM_DEBUG(llvm::dbgs() << "Applying child configurations\n");
           applyChildFormatTexts(&Style);
-
-          return Style;
         }
+        return Style;
+      }
 
-        LLVM_DEBUG(llvm::dbgs() << "Inherits parent configuration\n");
+      LLVM_DEBUG(llvm::dbgs() << "Inherits parent configuration\n");
 
-        // Reset inheritance of style
-        Style.InheritsParentConfig = false;
+      // Reset inheritance of style
+      Style.InheritsParentConfig = false;
 
-        ChildFormatTextToApply.emplace_back(std::move(*Text));
+      ChildFormatTextToApply.emplace_back(std::move(*Text));
 
-        // Breaking out of the inner loop, since we don't want to parse
-        // .clang-format AND _clang-format, if both exist. Then we continue the
-        // inner loop (parent directories) in search for the parent
-        // configuration.
-        break;
-      }
+      // Breaking out of the inner loop, since we don't want to parse
+      // .clang-format AND _clang-format, if both exist. Then we continue the
+      // outer loop (parent directories) in search for the parent
+      // configuration.
+      break;
     }
   }
+
   if (!UnsuitableConfigFiles.empty()) {
     return make_string_error("Configuration file(s) do(es) not support " +
                              getLanguageName(Style.Language) + ": " +
diff --git a/clang/tools/clang-format/ClangFormat.cpp b/clang/tools/clang-format/ClangFormat.cpp
index 829f85b93bc73c..d2e3d8d43aef21 100644
--- a/clang/tools/clang-format/ClangFormat.cpp
+++ b/clang/tools/clang-format/ClangFormat.cpp
@@ -398,8 +398,8 @@ class ClangFormatDiagConsumer : public DiagnosticConsumer {
 };
 
 // Returns true on error.
-static bool format(StringRef FileName) {
-  if (!OutputXML && Inplace && FileName == "-") {
+static bool format(StringRef FileName, bool IsSTDIN) {
+  if (!OutputXML && Inplace && IsSTDIN) {
     errs() << "error: cannot use -i when reading from stdin.\n";
     return false;
   }
@@ -423,7 +423,7 @@ static bool format(StringRef FileName) {
   if (InvalidBOM) {
     errs() << "error: encoding with unsupported byte order mark \""
            << InvalidBOM << "\" detected";
-    if (FileName != "-")
+    if (!IsSTDIN)
       errs() << " in file '" << FileName << "'";
     errs() << ".\n";
     return true;
@@ -432,7 +432,7 @@ static bool format(StringRef FileName) {
   std::vector<tooling::Range> Ranges;
   if (fillRanges(Code.get(), Ranges))
     return true;
-  StringRef AssumedFileName = (FileName == "-") ? AssumeFileName : FileName;
+  StringRef AssumedFileName = IsSTDIN ? AssumeFileName : FileName;
   if (AssumedFileName.empty()) {
     llvm::errs() << "error: empty filenames are not allowed\n";
     return true;
@@ -544,28 +544,23 @@ static void PrintVersion(raw_ostream &OS) {
 }
 
 // Dump the configuration.
-static int dumpConfig() {
-  StringRef FileName;
+static int dumpConfig(bool IsSTDIN) {
   std::unique_ptr<llvm::MemoryBuffer> Code;
-  if (FileNames.empty()) {
-    // We can't read the code to detect the language if there's no
-    // file name, so leave Code empty here.
-    FileName = AssumeFileName;
-  } else {
-    // Read in the code in case the filename alone isn't enough to
-    // detect the language.
+  // We can't read the code to detect the language if there's no file name.
+  if (!IsSTDIN) {
+    // Read in the code in case the filename alone isn't enough to detect the
+    // language.
     ErrorOr<std::unique_ptr<MemoryBuffer>> CodeOrErr =
         MemoryBuffer::getFileOrSTDIN(FileNames[0]);
     if (std::error_code EC = CodeOrErr.getError()) {
       llvm::errs() << EC.message() << "\n";
       return 1;
     }
-    FileName = (FileNames[0] == "-") ? AssumeFileName : FileNames[0];
     Code = std::move(CodeOrErr.get());
   }
   llvm::Expected<clang::format::FormatStyle> FormatStyle =
-      clang::format::getStyle(Style, FileName, FallbackStyle,
-                              Code ? Code->getBuffer() : "");
+      clang::format::getStyle(Style, IsSTDIN ? AssumeFileName : FileNames[0],
+                              FallbackStyle, Code ? Code->getBuffer() : "");
   if (!FormatStyle) {
     llvm::errs() << llvm::toString(FormatStyle.takeError()) << "\n";
     return 1;
@@ -596,8 +591,11 @@ int main(int argc, const char **argv) {
     return 0;
   }
 
+  if (FileNames.empty())
+    FileNames.push_back("-");
+
   if (DumpConfig)
-    return dumpConfig();
+    return dumpConfig(FileNames[0] == "-");
 
   if (!Files.empty()) {
     std::ifstream ExternalFileOfFiles{std::string(Files)};
@@ -610,11 +608,6 @@ int main(int argc, const char **argv) {
     errs() << "Clang-formating " << LineNo << " files\n";
   }
 
-  bool Error = false;
-  if (FileNames.empty()) {
-    Error = clang::format::format("-");
-    return Error ? 1 : 0;
-  }
   if (FileNames.size() != 1 &&
       (!Offsets.empty() || !Lengths.empty() || !LineRanges.empty())) {
     errs() << "error: -offset, -length and -lines can only be used for "
@@ -623,12 +616,13 @@ int main(int argc, const char **argv) {
   }
 
   unsigned FileNo = 1;
+  bool Error = false;
   for (const auto &FileName : FileNames) {
     if (Verbose) {
       errs() << "Formatting [" << FileNo++ << "/" << FileNames.size() << "] "
              << FileName << "\n";
     }
-    Error |= clang::format::format(FileName);
+    Error |= clang::format::format(FileName, FileName == "-");
   }
   return Error ? 1 : 0;
 }

[bhamiltoncx]

Seems like this regressed clang-format - -dump_config < path/to/objc_file.m. I filed #79023.