[clang] [clang][analyzer] Forward CTU-import failure conditions (PR #189064)

[Arseniy Zaostrovnykh via cfe-commits]

================
@@ -379,43 +406,98 @@ CrossTranslationUnitContext::getCrossTUDefinition(const VarDecl *VD,
                                   DisplayCTUProgress);
 }
 
-void CrossTranslationUnitContext::emitCrossTUDiagnostics(const IndexError &IE,
-                                                         SourceLocation Loc) {
+void CrossTranslationUnitContext::emitCrossTUDiagnostics(
+    const IndexError &IE, SourceLocation Loc) const {
   switch (IE.getCode()) {
   case index_error_code::missing_index_file:
+  case index_error_code::invocation_list_file_not_found:
+    // If the external def-map refers to source files, you must provide an
+    // invocation list file. Otherwise, CTU does not work at all, so you should
+    // check your build and analysis configuration.
     Context.getDiagnostics().Report(Loc, diag::err_ctu_error_opening)
         << IE.getFileName();
     return;
+
   case index_error_code::invalid_index_format:
     Context.getDiagnostics().Report(Loc, diag::err_extdefmap_parsing)
         << IE.getFileName() << IE.getLineNum();
     return;
+
   case index_error_code::multiple_definitions:
     Context.getDiagnostics().Report(Loc, diag::err_multiple_def_index)
         << IE.getLineNum();
     return;
+
   case index_error_code::triple_mismatch:
     Context.getDiagnostics().Report(Loc, diag::warn_ctu_incompat_triple)
-        << IE.getFileName() << IE.getTripleToName() << IE.getTripleFromName();
+        << IE.getFileName() << IE.getConfigToName() << IE.getConfigFromName();
     return;
-  case index_error_code::success:
-    llvm_unreachable("There should not be a success error. This case should "
-                     "have been handled by the caller.");
-    return;
-  case index_error_code::unspecified:
+
   case index_error_code::missing_definition:
+    // Ignore missing definitions because it is very common to have some symbols
+    // defined outside of the analysis scope: they may be defined in 3-rd party
+    // and standard libraries, generated code, and files excluded from the
+    // analysis.
+    // Even ignoring it with Ignored diagnostic might generate too much traffic.
+    return;
+
   case index_error_code::failed_import:
-  case index_error_code::failed_to_get_external_ast:
+  case index_error_code::unspecified:
+    // Not clear what happened exactly, but the outcome is a missing definition
+    // This is not a big deal, and is expected since ASTImporter is incomplete.
+    Context.getDiagnostics().Report(Loc, diag::warn_ctu_import_failure)
+        << Category->message(static_cast<int>(IE.getCode()));
+    return;
+
   case index_error_code::failed_to_generate_usr:
+    // This is unlikely, so it is worth looking into, hence an error.
+  case index_error_code::failed_to_get_external_ast:
+    // This is suspicious, since the external AST is mentioned in the external
+    // defmap, so it should exist.
+    Context.getDiagnostics().Report(Loc, diag::err_ctu_import_failure)
+        << Category->message(static_cast<int>(IE.getCode()));
+    return;
+
+  case index_error_code::load_threshold_reached:
+    // This is expected. It is still useful to be aware of, but it is normal
+    // operation.
+    Context.getDiagnostics().Report(Loc,
+                                    diag::remark_ctu_import_threshold_reached);
+    return;
+
   case index_error_code::lang_mismatch:
   case index_error_code::lang_dialect_mismatch:
-  case index_error_code::load_threshold_reached:
-  case index_error_code::invocation_list_ambiguous:
-  case index_error_code::invocation_list_file_not_found:
-  case index_error_code::invocation_list_empty:
+    // Similar to target triple mismatch.
+    Context.getDiagnostics().Report(Loc, diag::warn_ctu_incompat_lang)
+        << IE.getFileName() << IE.getConfigToName() << IE.getConfigFromName();
+    return;
+
   case index_error_code::invocation_list_wrong_format:
+  case index_error_code::invocation_list_empty:
+    // Without parsable invocation list, CTU cannot function.
+    Context.getDiagnostics().Report(Loc, diag::err_invlist_parsing)
+        << IE.getFileName() << IE.getLineNum();
+    return;
+
+  case index_error_code::invocation_list_ambiguous:
+    // For automatically generated invocation lists, it is common to list
+    // multiple invocations, if a file is compiled in multiple contexts. No need
+    // to block CTU because of this.
+    Context.getDiagnostics().Report(Loc, diag::warn_multiple_entries_invlist)
+        << IE.getFileName();
+    return;
----------------
necto wrote:

This is subjective, and I am open to reclassification as noted above.

I classified it as a warning because it requires user's attention: the analyzer could not choose the suitable invocation. This is something the user can fix.

I introduced only one diagnostics with "remark" level: reaching the import threshold. The difference from the user PoV is that you are more likely to not to do anything about the threshold: you set it to some "reasonable" level, and you expect that some definitions will never be imported. When it comes to invocation list, you might consider pre-filtering it, or changing build/analysis scope to avoid the duplication.

https://github.com/llvm/llvm-project/pull/189064