Add improved experimental ClangImporter diagnostics

Author: Nuri Amari

include/swift/AST/ASTContext.h

@@ -1123,6 +1123,9 @@ class ASTContext final {
   InheritedProtocolConformance *
   getInheritedConformance(Type type, ProtocolConformance *inherited);
 
+  /// Check if \p decl is included in LazyContexts.
+  bool isLazyContext(const DeclContext *decl);
+
   /// Get the lazy data for the given declaration.
   ///
   /// \param lazyLoader If non-null, the lazy loader to use when creating the

include/swift/AST/DiagnosticsClangImporter.def

@@ -96,5 +96,27 @@ WARNING(import_multiple_mainactor_attr,none,
 
 ERROR(module_map_not_found, none, "module map file '%0' not found", (StringRef))
 
+NOTE(macro_not_imported_unsupported_operator, none, "operator not supported in macro arithmetic", ())
+NOTE(macro_not_imported_unsupported_named_operator, none, "operator '%0' not supported in macro arithmetic", (StringRef))
+NOTE(macro_not_imported_invalid_string_literal, none, "invalid string literal", ())
+NOTE(macro_not_imported_invalid_numeric_literal, none, "invalid numeric literal", ())
+NOTE(macro_not_imported_unsupported_literal, none, "only numeric and string macro literals supported", ())
+NOTE(macro_not_imported_nested_cast, none, "non-null nested casts not supported", ())
+
+ERROR(macro_not_imported_function_like, none, "macro '%0' not imported: function like macros not supported", (StringRef))
+ERROR(macro_not_imported_unsupported_structure, none, "macro '%0' not imported: structure not supported", (StringRef))
+ERROR(macro_not_imported, none, "macro '%0' not imported", (StringRef))
+
+NOTE(return_type_not_imported, none, "return type not imported", ())
+NOTE(parameter_type_not_imported, none, "parameter '%0' not imported", (StringRef))
+NOTE(incomplete_type, none, "type '%0' is incomplete", (StringRef))
+NOTE(unsupported_builtin_type, none, "built-in type '%0' not supported", (StringRef))
+
+ERROR(record_field_not_imported, none, "field '%0' not imported", (StringRef))
+ERROR(invoked_func_not_imported, none, "function '%0' not imported", (StringRef))
+ERROR(record_method_not_imported, none, "method '%0' not imported", (StringRef))
+
+NOTE(forward_declaration_label, none, "type '%0' forward declared here", (StringRef))
+
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

include/swift/AST/LazyResolver.h

@@ -73,6 +73,12 @@ class alignas(void*) LazyMemberLoader {
 public:
   virtual ~LazyMemberLoader() = default;
 
+  /// Checks if diagnostics have been displayed for all
+  /// members of \p IDC that have DeclName matching \p N.
+  virtual bool
+  diagnosticsProducedForNamedMembers(const IterableDeclContext *IDC,
+                                     DeclBaseName N, uint64_t contextData) = 0;
+
   /// Populates a given decl \p D with all of its members.
   ///
   /// The implementation should add the members to D.

include/swift/ClangImporter/ClangImporter.h

@@ -40,6 +40,7 @@ namespace clang {
   class EnumDecl;
   class MacroInfo;
   class Module;
+  class ModuleMacro;
   class NamedDecl;
   class Sema;
   class TargetInfo;

lib/AST/ASTContext.cpp

@@ -2427,6 +2427,10 @@ ASTContext::getInheritedConformance(Type type, ProtocolConformance *inherited) {
   return result;
 }
 
+bool ASTContext::isLazyContext(const DeclContext *dc) {
+  return getImpl().LazyContexts[dc];
+}
+
 LazyContextData *ASTContext::getOrCreateLazyContextData(
                                                 const DeclContext *dc,
                                                 LazyMemberLoader *lazyLoader) {

lib/AST/NameLookup.cpp

@@ -1414,6 +1414,7 @@ TinyPtrVector<ValueDecl *>
 DirectLookupRequest::evaluate(Evaluator &evaluator,
                               DirectLookupDescriptor desc) const {
   const auto &name = desc.Name;
+  DeclBaseName baseName(name.getBaseName());
   const auto flags = desc.Options;
   auto *decl = desc.DC;
 
@@ -1440,7 +1441,36 @@ DirectLookupRequest::evaluate(Evaluator &evaluator,
   decl->addLoadedExtensions();
 
   auto &Table = *decl->LookupTable;
-  if (!useNamedLazyMemberLoading) {
+
+  bool lazilyTriggeredDiagnosticsOutstanding = false;
+  if (ctx.isLazyContext(decl)) {
+    auto ci = ctx.getOrCreateLazyIterableContextData(decl,
+                                                     /*lazyLoader=*/nullptr);
+    // If this lookup is already cached in the table or all members have already
+    // been loaded, and there are diagnostics that were suppressed during the
+    // previous loads, lazily load once more to produce these diagnostics. This
+    // occurs because diagnostics are suppressed during eager loading of
+    // members.
+    lazilyTriggeredDiagnosticsOutstanding =
+        ctx.LangOpts.EnableExperimentalClangImporterDiagnostics &&
+        ctx.LangOpts.NamedLazyMemberLoading &&
+        (!useNamedLazyMemberLoading || Table.isLazilyComplete(baseName)) &&
+        !ci->loader->diagnosticsProducedForNamedMembers(decl, baseName,
+                                                        ci->memberData);
+  }
+
+  if (lazilyTriggeredDiagnosticsOutstanding) {
+    // We are uninterested in the result of the import, just the diagnostics.
+    if (isa_and_nonnull<clang::RecordDecl>(decl->getClangDecl())) {
+      auto allFound = evaluateOrDefault(
+          ctx.evaluator,
+          ClangRecordMemberLookup({cast<StructDecl>(decl), name}), {});
+    } else if (isa<clang::ObjCContainerDecl>(decl->getClangDecl())) {
+      auto ci =
+          ctx.getOrCreateLazyIterableContextData(decl, /*lazyLoader=*/nullptr);
+      auto res = ci->loader->loadNamedMembers(decl, baseName, ci->memberData);
+    }
+  } else if (!useNamedLazyMemberLoading) {
     // Make sure we have the complete list of members (in this nominal and in
     // all extensions).
     (void)decl->getMembers();
@@ -1450,7 +1480,6 @@ DirectLookupRequest::evaluate(Evaluator &evaluator,
 
     Table.updateLookupTable(decl);
   } else if (!Table.isLazilyComplete(name.getBaseName())) {
-    DeclBaseName baseName(name.getBaseName());
 
     if (isa_and_nonnull<clang::NamespaceDecl>(decl->getClangDecl())) {
       // Namespaces will never have any members so we can just return whatever

lib/ClangImporter/ClangImporter.cpp

@@ -72,6 +72,7 @@
 #include "llvm/Support/YAMLTraits.h"
 #include <algorithm>
 #include <memory>
+#include <utility>
 
 using namespace swift;
 using namespace importer;
@@ -2256,6 +2257,109 @@ ClangModuleUnit *ClangImporter::Implementation::getClangModuleForDecl(
   return getWrapperForModule(M);
 }
 
+// TODO: Consolidate with importSourceLoc below
+SourceLoc ClangImporter::Implementation::convertSourceLocation(
+    const clang::SourceLocation &loc) {
+  return getBufferImporterForDiagnostics().resolveSourceLocation(
+      getClangASTContext().getSourceManager(), loc);
+}
+
+SwiftLookupTable::SingleEntry
+ClangImporter::Implementation::getDiagnosticTarget() {
+  return diagnosticTarget;
+}
+
+template <typename T>
+T ClangImporter::Implementation::withDiagnosticTargetIfNotAlreadyReported(
+    const SwiftLookupTable::SingleEntry &target, std::function<T()> fn) {
+  bool alreadyReported = reportedDiagnosticTargets.count(target);
+  if (!alreadyReported && !getEagerImportActive()) {
+    reportedDiagnosticTargets.insert(target);
+    return withDiagnosticTarget(target, fn);
+  }
+
+  return fn();
+}
+
+template <typename T>
+T ClangImporter::Implementation::withDiagnosticTarget(
+    const SwiftLookupTable::SingleEntry &target, std::function<T()> fn) {
+  assert(target);
+  // In some situations, the ClangImporter is invoked in a reentrant manner,
+  // resulting in a diagnosticTarget already being set. In such a case, we
+  // keep the outer diagnostic target.
+  llvm::SaveAndRestore<SwiftLookupTable::SingleEntry> sar{diagnosticTarget};
+  if (!diagnosticTarget) {
+    diagnosticTarget = target;
+  }
+  return fn();
+}
+
+bool ClangImporter::Implementation::isDiagnosticTarget(
+    const SwiftLookupTable::SingleEntry &targetCandidate) const {
+  return diagnosticTarget == targetCandidate;
+}
+bool ClangImporter::Implementation::isDiagnosticTarget(
+    const clang::NamedDecl *targetCandidate) const {
+  return diagnosticTarget.dyn_cast<clang::NamedDecl *>() == targetCandidate;
+}
+bool ClangImporter::Implementation::isDiagnosticTarget(
+    const clang::ModuleMacro *targetCandidate) const {
+  return diagnosticTarget.dyn_cast<clang::ModuleMacro *>() == targetCandidate;
+}
+bool ClangImporter::Implementation::isDiagnosticTarget(
+    const clang::MacroInfo *targetCandidate) const {
+  return diagnosticTarget.dyn_cast<clang::MacroInfo *>() == targetCandidate;
+}
+
+bool ClangImporter::Implementation::getEagerImportActive() {
+  return eagerImportActive;
+}
+
+void ClangImporter::Implementation::ifTargetMatchesReportErrorAndConsumeNotes(
+    const clang::NamedDecl *target, Diagnostic &&error,
+    const clang::SourceLocation &loc) {
+  if (!(SwiftContext.LangOpts.EnableExperimentalClangImporterDiagnostics &&
+        isDiagnosticTarget(target)))
+    return;
+
+  const SourceLoc convertedLoc = convertSourceLocation(loc);
+  diagnose(convertedLoc, error);
+  while (!pendingErrorNotes.empty()) {
+    auto pendingErrorNote = pendingErrorNotes.back();
+    if (!pendingErrorNote.diag.getLoc()) {
+      pendingErrorNote.diag.setLoc(convertedLoc);
+    }
+    if (isDiagnosticTarget(pendingErrorNote.target)) {
+      diagnose(pendingErrorNote.diag.getLoc(), pendingErrorNote.diag);
+    }
+    pendingErrorNotes.pop_back();
+  }
+}
+
+void ClangImporter::Implementation::addPendingErrorNote(Diagnostic &&note) {
+  addPendingErrorNote(std::move(note), clang::SourceLocation());
+}
+
+void ClangImporter::Implementation::addPendingErrorNote(
+    Diagnostic &&note, const clang::SourceLocation &loc) {
+  if (!(SwiftContext.LangOpts.EnableExperimentalClangImporterDiagnostics &&
+        getDiagnosticTarget()))
+    return;
+  note.setLoc(convertSourceLocation(loc));
+  pendingErrorNotes.push_back({note, getDiagnosticTarget()});
+}
+
+void ClangImporter::Implementation::applySourceLocationToNotesWithoutLocation(
+    const clang::SourceLocation &loc) {
+  const SourceLoc convertedLoc = convertSourceLocation(loc);
+  for (auto &pendingErrorNote : pendingErrorNotes) {
+    if (!pendingErrorNote.diag.getLoc()) {
+      pendingErrorNote.diag.setLoc(convertedLoc);
+    }
+  }
+}
+
 #pragma mark Source locations
 clang::SourceLocation
 ClangImporter::Implementation::exportSourceLoc(SourceLoc loc) {
@@ -3860,21 +3964,31 @@ void ClangImporter::Implementation::lookupValue(
     // If it's a Clang declaration, try to import it.
     if (auto clangDecl = entry.dyn_cast<clang::NamedDecl *>()) {
       bool isNamespace = isa<clang::NamespaceDecl>(clangDecl);
-      Decl *realDecl =
-          importDeclReal(clangDecl->getMostRecentDecl(), CurrentVersion,
-                         /*useCanonicalDecl*/ !isNamespace);
+      Decl *realDecl = withDiagnosticTargetIfNotAlreadyReported(
+          clangDecl->getMostRecentDecl(),
+          std::function<Decl *()>([&]() -> Decl * {
+            return importDeclReal(clangDecl->getMostRecentDecl(),
+                                  CurrentVersion,
+                                  /*useCanonicalDecl*/ !isNamespace);
+          }));
 
       if (!realDecl)
         continue;
       decl = cast<ValueDecl>(realDecl);
       if (!decl) continue;
     } else if (!name.isSpecial()) {
       // Try to import a macro.
-      if (auto modMacro = entry.dyn_cast<clang::ModuleMacro *>())
-        decl = importMacro(name.getBaseIdentifier(), modMacro);
-      else if (auto clangMacro = entry.dyn_cast<clang::MacroInfo *>())
-        decl = importMacro(name.getBaseIdentifier(), clangMacro);
-      else
+      if (auto modMacro = entry.dyn_cast<clang::ModuleMacro *>()) {
+        decl = withDiagnosticTargetIfNotAlreadyReported(
+            modMacro, std::function<ValueDecl *()>([&]() {
+              return importMacro(name.getBaseIdentifier(), modMacro);
+            }));
+      } else if (auto clangMacro = entry.dyn_cast<clang::MacroInfo *>()) {
+        decl = withDiagnosticTargetIfNotAlreadyReported(
+            clangMacro, std::function<ValueDecl *()>([&]() {
+              return importMacro(name.getBaseIdentifier(), clangMacro);
+            }));
+      } else
         llvm_unreachable("new kind of lookup table entry");
       if (!decl) continue;
     } else {
@@ -3954,6 +4068,9 @@ void ClangImporter::Implementation::lookupValue(
 void ClangImporter::Implementation::lookupVisibleDecls(
        SwiftLookupTable &table,
        VisibleDeclConsumer &consumer) {
+
+  llvm::SaveAndRestore<bool> sar(eagerImportActive, true);
+
   // Retrieve and sort all of the base names in this particular table.
   auto baseNames = table.allBaseNames();
   llvm::array_pod_sort(baseNames.begin(), baseNames.end());
@@ -4128,6 +4245,28 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
   return result;
 }
 
+bool ClangImporter::Implementation::diagnosticsProducedForNamedMembers(
+    const IterableDeclContext *IDC, DeclBaseName N, uint64_t contextData) {
+  auto *D = IDC->getDecl();
+  auto *DC = D->getInnermostDeclContext();
+  auto *CD = D->getClangDecl();
+  auto CMO = getClangSubmoduleForDecl(CD);
+  auto *nominal = DC->getSelfNominalTypeDecl();
+  auto effectiveClangContext = getEffectiveClangContext(nominal);
+  auto table = findLookupTable(*CMO);
+
+  for (auto entry :
+       table->lookup(SerializedSwiftName(N), effectiveClangContext)) {
+    if (auto member = entry.dyn_cast<clang::NamedDecl *>()) {
+      if (!reportedDiagnosticTargets.count(member)) {
+        return false;
+      }
+    }
+  }
+
+  return true;
+}
+
 TinyPtrVector<ValueDecl *>
 ClangImporter::Implementation::loadNamedMembers(
     const IterableDeclContext *IDC, DeclBaseName N, uint64_t contextData) {
@@ -4189,7 +4328,10 @@ ClangImporter::Implementation::loadNamedMembers(
     if (member->getDeclContext() != CDC) continue;
 
     SmallVector<Decl*, 4> tmp;
-    insertMembersAndAlternates(member, tmp);
+    withDiagnosticTargetIfNotAlreadyReported(
+        member, std::function<void()>(
+                    [&]() { insertMembersAndAlternates(member, tmp); }));
+
     for (auto *TD : tmp) {
       if (auto *V = dyn_cast<ValueDecl>(TD)) {
         // Skip ValueDecls if they import under different names.
@@ -4479,5 +4621,8 @@ ClangImporter::getEffectiveClangContext(const NominalTypeDecl *nominal) {
 }
 
 Decl *ClangImporter::importDeclDirectly(const clang::NamedDecl *decl) {
-  return Impl.importDecl(decl, Impl.CurrentVersion);
+  return Impl.withDiagnosticTargetIfNotAlreadyReported(
+      const_cast<clang::NamedDecl *>(decl), std::function<Decl *()>([&]() {
+        return Impl.importDecl(decl, Impl.CurrentVersion);
+      }));
 }