[libclang] Allow skipping warnings from all included files

[yvvan]

Summary

Updated clone for D48116 by Nikolai, now also adds the clang driver flag and the test for it.
If clang has plugins they are also affected by this filtering.

Depending on the included files and the used warning flags, e.g. -
Weverything, a huge number of warnings can be reported for included
files. As processing that many diagnostics comes with a performance
impact and not all clients are interested in those diagnostics, add a
flag to skip them.

[yvvan]

Restore missing tests

[ilya-biryukov]

include/clang/Driver/Options.td

1745

Why would we ever want to suppress diagnostics from *all* included files in the compiler mode?

That seems more appropriate in the tooling mode (e.g. IDE integration, etc.). However, that could probably be done without modifications of core clang bits, e.g. by writing a DiagnosticsConsumer that ignores diagnostics coming from outside the main file.

[yvvan]

I already mentioned in the review inherited by this one that this way covers also loaded plugins with different consumers. So let's assume that driver loads clang-tidy and some other plugins and runs over each file in the project.
It makes sense not to include clang-tidy diagnostics coming from headers, at least for third-party headers (you can still run clang over specific headers themselves to get diagnostics for them).

[ilya-biryukov]

In D49794#1182078, @yvvan wrote:

I already mentioned in the review inherited by this one that this way covers also loaded plugins with different consumers. So let's assume that driver loads clang-tidy and some other plugins and runs over each file in the project.
It makes sense not to include clang-tidy diagnostics coming from headers, at least for third-party headers (you can still run clang over specific headers themselves to get diagnostics for them).

For third-party headers, sure (as I mentioned before, we already use -isystem include directive for third party libs to achieve the same thing). But ignoring diagnostics from all the headers does not seem particularly useful.

[yvvan]

And we already saw, that -isystem is not the best choice for that. And by the way - clang-tidy has this filtering in consumer which does not exist in it's plugin-mode so it spits all system diagnostics all the time...

[yvvan]

Anyways, if c++ part does not seem relevant I'm fine if we only have libclang part from D48116.

BTW I don't remember if you answered something to my suggestion of adding flag -ithird-party as an alternative to -isystem which does not lock files and helps to filter diagnostics, etc.

[ilya-biryukov]

In D49794#1182220, @yvvan wrote:

And we already saw, that -isystem is not the best choice for that.

Are you referring to the file-locking on Windows?
Any other reasons why the -isystem trick might be non-ideal?

And by the way - clang-tidy has this filtering in consumer which does not exist in it's plugin-mode so it spits all system diagnostics all the time...

I would still argue against adding this option in the current form to the compiler. I don't see how that might be useful to non-clang-tidy, non-editor-integration users.

What's the 'plugin mode' of clang-tidy? I don't seem to able to Google it :-)

[yvvan]

In D49794#1182272, @ilya-biryukov wrote:

In D49794#1182220, @yvvan wrote:

And we already saw, that -isystem is not the best choice for that.

Are you referring to the file-locking on Windows?
Any other reasons why the -isystem trick might be non-ideal?

File locking is the first one. Another one comes with "plugin mode" of tidy.

And by the way - clang-tidy has this filtering in consumer which does not exist in it's plugin-mode so it spits all system diagnostics all the time...

I would still argue against adding this option in the current form to the compiler. I don't see how that might be useful to non-clang-tidy, non-editor-integration users.

What's the 'plugin mode' of clang-tidy? I don't seem to able to Google it :-)

Clang tidy is not only a standalone tool but also a plugin. It's almost never used this way (but we do that in Qt Creator to combine it with Clazy) and it also seems that some of the recent checks are added only to standalone tool (which is easy to fix).
Being loaded into clang as plugin it does not support most of clang-tidy command line options and is not able to filter system includes. I'm not sure it's possible to fix both of these issues (and probably some other).

[ilya-biryukov]

In D49794#1182296, @yvvan wrote:

Clang tidy is not only a standalone tool but also a plugin. It's almost never used this way (but we do that in Qt Creator to combine it with Clazy) and it also seems that some of the recent checks are added only to standalone tool (which is easy to fix).
Being loaded into clang as plugin it does not support most of clang-tidy command line options and is not able to filter system includes. I'm not sure it's possible to fix both of these issues (and probably some other).

Oh, I see, thanks for the pointers. Not related to this discussion, it seems reasonable to allow all the same options to be passed into clang-tidy plugin, not sure if that's easy or hard in principle to fix it.

The added option seems like a workaround for a bug in clang-tidy and a windows locking bug (which is blocker for tools like clangd and libclang-based integrations, IMO). I would still vouch for fixing the bugs than adding flags that try to workaround those bugs.

include/clang-c/Index.h

enum CXTranslationUnit_Flags {
   CXTranslationUnit_SingleFileParse = 0x400,
 
   /**
    * Used in combination with CXTranslationUnit_SkipFunctionBodies to
    * constrain the skipping of function bodies to the preamble.
    *
    * The function bodies of the main file are not skipped.
    */
-  CXTranslationUnit_LimitSkipFunctionBodiesToPreamble = 0x800
+  CXTranslationUnit_LimitSkipFunctionBodiesToPreamble = 0x800,
+
+  /**
+   * Used to indicate that non-errors from included files should be ignored.
+   *
+   * If set, clang_getDiagnosticSetFromTU() will not report e.g. warnings from
+   * included files anymore. This speeds up clang_getDiagnosticSetFromTU() for
+   * the case where these warnings are not of interest, as for an IDE for
+   * example, which typically shows only the diagnostics in the main file.
+   */
+  CXTranslationUnit_IgnoreNonErrorsFromIncludedFiles = 0x1000
 };
 
 /**
  * Returns the set of flags that is suitable for parsing a translation
  * unit that is being edited.
  *
  * The set of flags returned provide options for \c clang_parseTranslationUnit()
  * to indicate that the translation unit is likely to be reparsed many times,

include/clang/Basic/Diagnostic.h

private:
   unsigned char AllExtensionsSilenced = 0;
 
   // Suppress diagnostics after a fatal error?
   bool SuppressAfterFatalError = true;
 
   // Suppress all diagnostics.
   bool SuppressAllDiagnostics = false;
 
+  // Suppress non-errors from all included files.
+  bool SuppressNonErrorsFromIncludedFiles = false;
+
   // Elide common types of templates.
   bool ElideType = true;
 
   // Print a tree when comparing templates.
   bool PrintTemplateTree = false;
 
   // Color printing is enabled.
   bool ShowColors = false;
public:
   /// Suppress all diagnostics, to silence the front end when we 
   /// know that we don't want any more diagnostics to be passed along to the
   /// client
   void setSuppressAllDiagnostics(bool Val = true) { 
     SuppressAllDiagnostics = Val; 
   }
   bool getSuppressAllDiagnostics() const { return SuppressAllDiagnostics; }
 
+  void setSuppressNonErrorsFromIncludedFiles(bool Val = true) {
+    SuppressNonErrorsFromIncludedFiles = Val;
+  }
+
   /// Set type eliding, to skip outputting same types occurring in
   /// template types.
   void setElideType(bool Val = true) { ElideType = Val; }
   bool getElideType() { return ElideType; }
  
   /// Set tree printing, to outputting the template difference in a
   /// tree format.
   void setPrintTemplateTree(bool Val = false) { PrintTemplateTree = Val; }

include/clang/Basic/DiagnosticOptions.def

 #endif
 
 #ifndef SEMANTIC_ENUM_DIAGOPT
 #  define SEMANTIC_ENUM_DIAGOPT(Name, Type, Bits, Default) \
      ENUM_DIAGOPT(Name, Type, Bits, Default)
 #endif
 
 SEMANTIC_DIAGOPT(IgnoreWarnings, 1, 0)   /// -w
+DIAGOPT(SuppressNonErrorsFromIncludedFiles, 1, 0)  /// -fsuppress-non-errors-from-included-files
 DIAGOPT(NoRewriteMacros, 1, 0)  /// -Wno-rewrite-macros
 DIAGOPT(Pedantic, 1, 0)         /// -pedantic
 DIAGOPT(PedanticErrors, 1, 0)   /// -pedantic-errors
 DIAGOPT(ShowColumn, 1, 1)       /// Show column number on diagnostics.
 DIAGOPT(ShowLocation, 1, 1)     /// Show source location information.
 DIAGOPT(AbsolutePath, 1, 0)     /// Use absolute paths.
 DIAGOPT(ShowCarets, 1, 1)       /// Show carets in diagnostics.
 DIAGOPT(ShowFixits, 1, 1)       /// Show fixit information.

include/clang/Driver/Options.td

def fno_debug_types_section: Flag<["-"], "fno-debug-types-section">, Group<f_Group>,
   Flags<[CC1Option]>;
 def fsplit_dwarf_inlining: Flag <["-"], "fsplit-dwarf-inlining">, Group<f_Group>,
   Flags<[CC1Option]>, HelpText<"Provide minimal debug info in the object/executable to facilitate online symbolication/stack traces in the absence of .dwo/.dwp files when using Split DWARF">;
 def fno_split_dwarf_inlining: Flag<["-"], "fno-split-dwarf-inlining">, Group<f_Group>,
   Flags<[CC1Option]>;
 def fdebug_prefix_map_EQ
   : Joined<["-"], "fdebug-prefix-map=">, Group<f_Group>, Flags<[CC1Option]>,
     HelpText<"remap file source paths in debug info">;
+def fsuppress_non_errors_from_included_files : Flag<["-"], "fsuppress-non-errors-from-included-files">,
+  Group<f_Group>, Flags<[CC1Option]>;
 def g_Flag : Flag<["-"], "g">, Group<g_Group>,
   HelpText<"Generate source-level debug information">;
 def gline_tables_only : Flag<["-"], "gline-tables-only">, Group<gN_Group>,
   Flags<[CoreOption]>, HelpText<"Emit debug line number tables only">;
 def gmlt : Flag<["-"], "gmlt">, Alias<gline_tables_only>;
 def g0 : Flag<["-"], "g0">, Group<gN_Group>;
 def g1 : Flag<["-"], "g1">, Group<gN_Group>, Alias<gline_tables_only>;
 def g2 : Flag<["-"], "g2">, Group<gN_Group>;

lib/Basic/DiagnosticIDs.cpp

DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc,
 
   // If -Wfatal-errors is enabled, map errors to fatal unless explicitly
   // disabled.
   if (Result == diag::Severity::Error) {
     if (State->ErrorsAsFatal && !Mapping.hasNoErrorAsFatal())
       Result = diag::Severity::Fatal;
   }
 
+  // If requested, ignore non-errors from all included files.
+  if (Diag.SuppressNonErrorsFromIncludedFiles &&
+      Result <= diag::Severity::Warning && Loc.isValid() &&
+      !Diag.getSourceManager().isInMainFile(
+          Diag.getSourceManager().getExpansionLoc(Loc))) {
+    return diag::Severity::Ignored;
+  }
+
   // Custom diagnostics always are emitted in system headers.
   bool ShowInSystemHeader =
       !GetDiagInfo(DiagID) || GetDiagInfo(DiagID)->WarnShowInSystemHeader;
 
   // If we are in a system header, we ignore it. We look at the diagnostic class
   // because we also want to ignore extensions and warnings in -Werror and
   // -pedantic-errors modes, which *map* warnings/extensions to errors.
   if (State->SuppressSystemWarnings && !ShowInSystemHeader && Loc.isValid() &&

lib/Basic/Warnings.cpp

Diags.Report(diag::warn_unknown_diag_option)
     << !Suggestion.empty() << (Prefix.str() += Suggestion);
 }
 
 void clang::ProcessWarningOptions(DiagnosticsEngine &Diags,
                                   const DiagnosticOptions &Opts,
                                   bool ReportDiags) {
   Diags.setSuppressSystemWarnings(true);  // Default to -Wno-system-headers
   Diags.setIgnoreAllWarnings(Opts.IgnoreWarnings);
+  Diags.setSuppressNonErrorsFromIncludedFiles(Opts.SuppressNonErrorsFromIncludedFiles);
   Diags.setShowOverloads(Opts.getShowOverloads());
 
   Diags.setElideType(Opts.ElideType);
   Diags.setPrintTemplateTree(Opts.ShowTemplateTree);
   Diags.setShowColors(Opts.ShowColors);
  
   // Handle -ferror-limit
   if (Opts.ErrorLimit)

lib/Frontend/CompilerInvocation.cpp

bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
                                 bool DefaultDiagColor, bool DefaultShowOpt) {
   bool Success = true;
 
   Opts.DiagnosticLogFile = Args.getLastArgValue(OPT_diagnostic_log_file);
   if (Arg *A =
           Args.getLastArg(OPT_diagnostic_serialized_file, OPT__serialize_diags))
     Opts.DiagnosticSerializationFile = A->getValue();
   Opts.IgnoreWarnings = Args.hasArg(OPT_w);
+  Opts.SuppressNonErrorsFromIncludedFiles =
+      Args.hasArg(OPT_fsuppress_non_errors_from_included_files);
   Opts.NoRewriteMacros = Args.hasArg(OPT_Wno_rewrite_macros);
   Opts.Pedantic = Args.hasArg(OPT_pedantic);
   Opts.PedanticErrors = Args.hasArg(OPT_pedantic_errors);
   Opts.ShowCarets = !Args.hasArg(OPT_fno_caret_diagnostics);
   Opts.ShowColors = parseShowColorsArgs(Args, DefaultDiagColor);
   Opts.ShowColumn = Args.hasFlag(OPT_fshow_column,
                                  OPT_fno_show_column,
                                  /*Default=*/true);

test/Frontend/Inputs/header-with-warning.h

+static void f0(void) {}

test/Frontend/warnings-ignore-from-includes.cpp

+#include "header-with-warning.h"
+
+static void f1(void) {}
+
+// RUN: %clang_cc1 -I%S/Inputs/ -Wunused-function -fsuppress-non-errors-from-included-files %s 2>&1 | FileCheck %s
+// CHECK-NOT: warning: unused function 'f0'
+// CHECK: warning: unused function 'f1'

test/Index/ignore-warnings-from-headers.h

+void f(int unusedInHeader) {}

test/Index/ignore-warnings-from-headers.cpp

+#include "ignore-warnings-from-headers.h"
+
+void g(int unusedInMainFile) {}
+
+// RUN: env CINDEXTEST_IGNORE_NONERRORS_FROM_INCLUDED_FILES=1 c-index-test -test-load-source function %s -Wunused-parameter 2>&1 | FileCheck %s
+// CHECK-NOT: warning: unused parameter 'unusedInHeader'
+// CHECK: warning: unused parameter 'unusedInMainFile'

tools/c-index-test/c-index-test.c

static unsigned getDefaultParsingOptions() {
   if (getenv("CINDEXTEST_COMPLETION_BRIEF_COMMENTS"))
     options |= CXTranslationUnit_IncludeBriefCommentsInCodeCompletion;
   if (getenv("CINDEXTEST_CREATE_PREAMBLE_ON_FIRST_PARSE"))
     options |= CXTranslationUnit_CreatePreambleOnFirstParse;
   if (getenv("CINDEXTEST_KEEP_GOING"))
     options |= CXTranslationUnit_KeepGoing;
   if (getenv("CINDEXTEST_LIMIT_SKIP_FUNCTION_BODIES_TO_PREAMBLE"))
     options |= CXTranslationUnit_LimitSkipFunctionBodiesToPreamble;
+  if (getenv("CINDEXTEST_IGNORE_NONERRORS_FROM_INCLUDED_FILES"))
+    options |= CXTranslationUnit_IgnoreNonErrorsFromIncludedFiles;
 
   return options;
 }
 
 static void ModifyPrintingPolicyAccordingToEnv(CXPrintingPolicy Policy) {
   struct Mapping {
     const char *name;
     enum CXPrintingPolicyProperty property;

tools/libclang/CIndex.cpp

clang_parseTranslationUnit_Impl(CXIndex CIdx, const char *source_filename,
 
   // Configure the diagnostics.
   IntrusiveRefCntPtr<DiagnosticsEngine>
     Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions));
 
   if (options & CXTranslationUnit_KeepGoing)
     Diags->setSuppressAfterFatalError(false);
 
+  if (options & CXTranslationUnit_IgnoreNonErrorsFromIncludedFiles)
+      Diags->setSuppressNonErrorsFromIncludedFiles(true);
+
   // Recover resources if we crash before exiting this function.
   llvm::CrashRecoveryContextCleanupRegistrar<DiagnosticsEngine,
     llvm::CrashRecoveryContextReleaseRefCleanup<DiagnosticsEngine> >
     DiagCleanup(Diags.get());
 
   std::unique_ptr<std::vector<ASTUnit::RemappedFile>> RemappedFiles(
       new std::vector<ASTUnit::RemappedFile>());