Merge pull request swiftlang#82724 from hamishknight/diag-it

Author: hamishknight

include/swift/AST/DiagnosticEngine.h

@@ -618,7 +618,10 @@ namespace swift {
 
     /// Figure out the Behavior for the given diagnostic, taking current
     /// state such as fatality into account.
-    DiagnosticBehavior determineBehavior(const Diagnostic &diag);
+    DiagnosticBehavior determineBehavior(const Diagnostic &diag) const;
+
+    /// Updates the diagnostic state for a diagnostic to emit.
+    void updateFor(DiagnosticBehavior behavior);
 
     bool hadAnyError() const { return anyErrorOccurred; }
     bool hasFatalErrorOccurred() const { return fatalErrorOccurred; }
@@ -646,7 +649,7 @@ namespace swift {
 
     /// Returns a Boolean value indicating whether warnings belonging to the
     /// diagnostic group identified by `id` should be escalated to errors.
-    bool getWarningsAsErrorsForDiagGroupID(DiagGroupID id) {
+    bool getWarningsAsErrorsForDiagGroupID(DiagGroupID id) const {
       return warningsAsErrors[(unsigned)id];
     }
 

lib/AST/DiagnosticEngine.cpp

@@ -1287,7 +1287,8 @@ llvm::cl::opt<bool> AssertOnError("swift-diagnostics-assert-on-error",
 llvm::cl::opt<bool> AssertOnWarning("swift-diagnostics-assert-on-warning",
                                     llvm::cl::init(false));
 
-DiagnosticBehavior DiagnosticState::determineBehavior(const Diagnostic &diag) {
+DiagnosticBehavior
+DiagnosticState::determineBehavior(const Diagnostic &diag) const {
   // We determine how to handle a diagnostic based on the following rules
   //   1) Map the diagnostic to its "intended" behavior, applying the behavior
   //      limit for this particular emission
@@ -1334,21 +1335,23 @@ DiagnosticBehavior DiagnosticState::determineBehavior(const Diagnostic &diag) {
     if (suppressRemarks)
       lvl = DiagnosticBehavior::Ignore;
   }
+  return lvl;
+}
 
-  //   5) Update current state for use during the next diagnostic
-  if (lvl == DiagnosticBehavior::Fatal) {
+void DiagnosticState::updateFor(DiagnosticBehavior behavior) {
+  // Update current state for use during the next diagnostic
+  if (behavior == DiagnosticBehavior::Fatal) {
     fatalErrorOccurred = true;
     anyErrorOccurred = true;
-  } else if (lvl == DiagnosticBehavior::Error) {
+  } else if (behavior == DiagnosticBehavior::Error) {
     anyErrorOccurred = true;
   }
 
   ASSERT((!AssertOnError || !anyErrorOccurred) && "We emitted an error?!");
-  ASSERT((!AssertOnWarning || (lvl != DiagnosticBehavior::Warning)) &&
+  ASSERT((!AssertOnWarning || (behavior != DiagnosticBehavior::Warning)) &&
          "We emitted a warning?!");
 
-  previousBehavior = lvl;
-  return lvl;
+  previousBehavior = behavior;
 }
 
 void DiagnosticEngine::flushActiveDiagnostic() {
@@ -1393,6 +1396,8 @@ std::optional<DiagnosticInfo>
 DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic,
                                               bool includeDiagnosticName) {
   auto behavior = state.determineBehavior(diagnostic);
+  state.updateFor(behavior);
+
   if (behavior == DiagnosticBehavior::Ignore)
     return std::nullopt;
 

lib/Sema/TypeCheckExpr.cpp

@@ -774,14 +774,30 @@ Expr *CallerSideDefaultArgExprRequest::evaluate(
   if (!TypeChecker::typeCheckParameterDefault(initExpr, dc, paramTy,
                                               param->isAutoClosure(),
                                               /*atCallerSide=*/true)) {
-    if (param->hasDefaultExpr()) {
+    auto isSimpleLiteral = [&]() -> bool {
+      switch (param->getDefaultArgumentKind()) {
+#define MAGIC_IDENTIFIER(NAME, STRING) \
+      case DefaultArgumentKind::NAME: return true;
+#include "swift/AST/MagicIdentifierKinds.def"
+      case DefaultArgumentKind::NilLiteral:
+      case DefaultArgumentKind::EmptyArray:
+      case DefaultArgumentKind::EmptyDictionary:
+        return true;
+      default:
+        return false;
+      }
+    };
+    if (param->hasDefaultExpr() && isSimpleLiteral()) {
       // HACK: If we were unable to type-check the default argument in context,
       // then retry by type-checking it within the parameter decl, which should
       // also fail. This will present the user with a better error message and
       // allow us to avoid diagnosing on each call site.
+      // Note we can't do this for expression macros since name lookup may
+      // differ at the call side vs the declaration. We can however do it for
+      // simple literals.
       transaction.abort();
       (void)param->getTypeCheckedDefaultExpr();
-      assert(ctx.Diags.hadAnyError());
+      ASSERT(ctx.Diags.hadAnyError());
     }
     return new (ctx) ErrorExpr(initExpr->getSourceRange(), paramTy);
   }

test/Macros/rdar154771596.swift

@@ -0,0 +1,28 @@
+// REQUIRES: swift_swift_parser
+
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %host-build-swift -swift-version 5 -emit-library -o %t/%target-library-name(MacroDefinition) -module-name=MacroDefinition %S/Inputs/syntax_macro_definitions.swift -g -no-toolchain-stdlib-rpath
+
+// RUN: %target-swift-frontend -emit-module %t/Lib.swift -module-name Lib -emit-module-path %t/Lib.swiftmodule
+
+// RUN: %target-swift-frontend -typecheck -verify -swift-version 5 -load-plugin-library %t/%target-library-name(MacroDefinition) -I %t %t/a.swift -primary-file %t/b.swift
+
+//--- Lib.swift
+
+@freestanding(expression)
+public macro magicFile() -> String = #externalMacro(module: "MacroDefinition", type: "MagicFileMacro")
+
+//--- a.swift
+
+import Lib
+
+func foo(x: String = #magicFile) {}
+
+//--- b.swift
+
+// We're missing the necessary import in this file, make sure we diagnose.
+func bar() {
+  foo() // expected-error {{no macro named 'magicFile'}}
+}