[clang-format] Suppress diagnostics on second parse

[HazardyKnusperkeks]

Summary

This is the follow up to D93844.

Suppress the diagnostics when applying the child configurations, since they were printed on the first parse.

[HazardyKnusperkeks]

clang/include/clang/Format/Format.h

3342–3346

On my code I would do that change too.

This way we would get all the changes to the signature without manual work.

[njames93]

I did have another plan for a different way to go about this, not sure if its really any better though.
It involved building a vector of the edits the config would apply to its parent. Then after the parent is parsed just applying those edits.
It results in only 1 pass over the config file however it's a little more involved to get it to work.

clang/include/clang/Format/Format.h

3339

I know this is sufficient for the purpose of this patch, however it doesn't make sense to accept take the handler parameter without also taking a void* for its context.

[HazardyKnusperkeks]

In D96760#2565171, @njames93 wrote:

I did have another plan for a different way to go about this, not sure if its really any better though.
It involved building a vector of the edits the config would apply to its parent. Then after the parent is parsed just applying those edits.
It results in only 1 pass over the config file however it's a little more involved to get it to work.

But then I need to rewrite the complete parsing process? Because in a normal style I can not parse, because it is not apparent which entries are changed because of the config, or do I oversee something?

In D93844#2536354, @njames93 wrote:

As a follow up it may be wise to pass a diag handler to parseConfiguration as when we parse it a second time, we probably want to disregard any warnings (like unknown key) detected as they will have been printed on the first pass.

And this is what you proposed. :)

clang/include/clang/Format/Format.h

3339

Will do.

[HazardyKnusperkeks]

Added DiagHandlerCtxt.

[njames93]

In D96760#2569151, @HazardyKnusperkeks wrote:

In D96760#2565171, @njames93 wrote:

I did have another plan for a different way to go about this, not sure if its really any better though.
It involved building a vector of the edits the config would apply to its parent. Then after the parent is parsed just applying those edits.
It results in only 1 pass over the config file however it's a little more involved to get it to work.

But then I need to rewrite the complete parsing process? Because in a normal style I can not parse, because it is not apparent which entries are changed because of the config, or do I oversee something?

In D93844#2536354, @njames93 wrote:

As a follow up it may be wise to pass a diag handler to parseConfiguration as when we parse it a second time, we probably want to disregard any warnings (like unknown key) detected as they will have been printed on the first pass.

And this is what you proposed. :)

The diaghandler is a perfectly good solution, I was just spit balling ideas. But it is possible to store changes, though not much is gained for this use case.

[HazardyKnusperkeks]

Ping.

[HazardyKnusperkeks]

Rebased.

[njames93]

LGTM, but see what @MyDeveloperDay has to say.

[MyDeveloperDay]

LGTM

clang/include/clang/Format/Format.h

 #ifndef LLVM_CLANG_FORMAT_FORMAT_H
 #define LLVM_CLANG_FORMAT_FORMAT_H
 
 #include "clang/Basic/LangOptions.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Inclusions/IncludeStyle.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Regex.h"
+#include "llvm/Support/SourceMgr.h"
 #include <system_error>
 
 namespace llvm {
 namespace vfs {
 class FileSystem;
 }
 } // namespace llvm
 

 
   static FormatStyleSet BuildStyleSetFromConfiguration(
       const FormatStyle &MainStyle,
       const std::vector<FormatStyle> &ConfigurationStyles);
 
 private:
   FormatStyleSet StyleSet;
 
-  friend std::error_code parseConfiguration(llvm::MemoryBufferRef Config,
-                                            FormatStyle *Style,
-                                            bool AllowUnknownOptions);
+  friend std::error_code
+  parseConfiguration(llvm::MemoryBufferRef Config, FormatStyle *Style,
+                     bool AllowUnknownOptions,
+                     llvm::SourceMgr::DiagHandlerTy DiagHandler);
 };
 
 /// Returns a format style complying with the LLVM coding standards:
 /// http://llvm.org/docs/CodingStandards.html.
 FormatStyle getLLVMStyle(
     FormatStyle::LanguageKind Language = FormatStyle::LanguageKind::LK_Cpp);
 
 /// Returns a format style complying with one of Google's style guides:

 ///
 /// The FormatStyleSet of Style is reset.
 ///
 /// When ``BasedOnStyle`` is not present, options not present in the YAML
 /// document, are retained in \p Style.
 ///
 /// If AllowUnknownOptions is true, no errors are emitted if unknown
 /// format options are occured.
-std::error_code parseConfiguration(llvm::MemoryBufferRef Config,
-                                   FormatStyle *Style,
-                                   bool AllowUnknownOptions = false);
+///
+/// If set all diagnostics are emitted through the DiagHandler.
+std::error_code
+parseConfiguration(llvm::MemoryBufferRef Config, FormatStyle *Style,
+                   bool AllowUnknownOptions = false,
+                   llvm::SourceMgr::DiagHandlerTy DiagHandler = nullptr);
 
 /// Like above but accepts an unnamed buffer.
 inline std::error_code parseConfiguration(StringRef Config, FormatStyle *Style,
                                           bool AllowUnknownOptions = false) {
   return parseConfiguration(llvm::MemoryBufferRef(Config, "YAML"), Style,
                             AllowUnknownOptions);
 }
 
 /// Gets configuration in a YAML string.
 std::string configurationAsText(const FormatStyle &Style);
 
 /// Returns the replacements necessary to sort all ``#include`` blocks
 /// that are affected by ``Ranges``.
 tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code,
                                    ArrayRef<tooling::Range> Ranges,

clang/lib/Format/Format.cpp

if (Name.equals_lower("llvm")) {
     return false;
   }
 
   Style->Language = Language;
   return true;
 }
 
 std::error_code parseConfiguration(llvm::MemoryBufferRef Config,
-                                   FormatStyle *Style,
-                                   bool AllowUnknownOptions) {
+                                   FormatStyle *Style, bool AllowUnknownOptions,
+                                   llvm::SourceMgr::DiagHandlerTy DiagHandler) {
   assert(Style);
   FormatStyle::LanguageKind Language = Style->Language;
   assert(Language != FormatStyle::LK_None);
   if (Config.getBuffer().trim().empty())
     return make_error_code(ParseError::Error);
   Style->StyleSet.Clear();
   std::vector<FormatStyle> Styles;
-  llvm::yaml::Input Input(Config);
+  llvm::yaml::Input Input(Config, /*Ctxt=*/nullptr, DiagHandler,
+                          /*DiagHandlerCtxt=*/nullptr);
   // DocumentListTraits<vector<FormatStyle>> uses the context to get default
   // values for the fields, keys for which are missing from the configuration.
   // Mapping also uses the context to get the language to find the correct
   // base style.
   Input.setContext(Style);
   Input.setAllowUnknownKeys(AllowUnknownOptions);
   Input >> Styles;
   if (Input.error())
llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName,
   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");
 
+  auto dropDiagnosticHandler = [](const llvm::SMDiagnostic &, void *) {};
+
   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) {
       continue;
     }
for (const auto &F : FilesToLookFor) {
 
         if (!Style.InheritsParentConfig) {
           if (ChildFormatTextToApply.empty())
             return Style;
 
           LLVM_DEBUG(llvm::dbgs() << "Applying child configurations\n");
 
           for (const auto& MemBuf : llvm::reverse(ChildFormatTextToApply)){
-            auto Ec = parseConfiguration(*MemBuf, &Style, AllowUnknownOptions);
+            auto Ec = parseConfiguration(*MemBuf, &Style, AllowUnknownOptions,
+                                         dropDiagnosticHandler);
             // It was already correctly parsed.
             assert(!Ec);
             static_cast<void>(Ec);
           }
 
           return Style;
         }
 
return make_string_error("Configuration file(s) do(es) not support " +
                              UnsuitableConfigFiles);
 
   if (!ChildFormatTextToApply.empty()) {
     assert(ChildFormatTextToApply.size() == 1);
 
     LLVM_DEBUG(llvm::dbgs()
                << "Applying child configuration on fallback style\n");
 
-    auto Ec = parseConfiguration(*ChildFormatTextToApply.front(),
-                                 &FallbackStyle, AllowUnknownOptions);
+    auto Ec =
+        parseConfiguration(*ChildFormatTextToApply.front(), &FallbackStyle,
+                           AllowUnknownOptions, dropDiagnosticHandler);
     // It was already correctly parsed.
     assert(!Ec);
     static_cast<void>(Ec);
   }
 
   return FallbackStyle;
 }
 
 } // namespace format
 } // namespace clang