[Macros] Diagnose errors where a macro is used without the '#'.

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -6794,6 +6794,8 @@ ERROR(macro_in_nested,none,
 ERROR(macro_without_context,none,
       "macro %0 must declare its applicable contexts (e.g., '@expression')",
        (DeclName))
+ERROR(macro_expansion_missing_pound,none,
+      "expansion of macro %0 requires leading '#'", (DeclName))
 
 //------------------------------------------------------------------------------
 // MARK: Move Only Errors

include/swift/Sema/CSFix.h

@@ -415,6 +415,9 @@ enum class FixKind : uint8_t {
   /// For example `.a(let x), .b(let x)` where `x` gets bound to different
   /// types.
   RenameConflictingPatternVariables,
+
+  /// Macro without leading #.
+  MacroMissingPound,
 };
 
 class ConstraintFix {
@@ -3228,6 +3231,32 @@ class RenameConflictingPatternVariables final
   }
 };
 
+class MacroMissingPound final : public ConstraintFix {
+  MacroDecl *macro;
+
+  MacroMissingPound(ConstraintSystem &cs, MacroDecl *macro,
+                    ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::MacroMissingPound, locator),
+        macro(macro) { }
+
+public:
+  std::string getName() const override { return "macro missing pound"; }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
+    return diagnose(*commonFixes.front().first);
+  }
+
+  static MacroMissingPound *
+  create(ConstraintSystem &cs, MacroDecl *macro,
+         ConstraintLocator *locator);
+
+  static bool classof(ConstraintFix *fix) {
+    return fix->getKind() == FixKind::MacroMissingPound;
+  }
+};
+
 } // end namespace constraints
 } // end namespace swift
 

include/swift/Sema/ConstraintLocator.h

@@ -292,6 +292,9 @@ class ConstraintLocator : public llvm::FoldingSetNode {
   /// Determine whether this locator is for a result builder body result type.
   bool isForResultBuilderBodyResult() const;
 
+  /// Determine whether this locator is for a macro expansion.
+  bool isForMacroExpansion() const;
+
   /// Determine whether this locator points directly to a given expression.
   template <typename E> bool directlyAt() const {
     if (auto *expr = getAnchor().dyn_cast<Expr *>())

lib/Sema/CSDiagnostics.cpp

@@ -8496,3 +8496,9 @@ bool ConflictingPatternVariables::diagnoseAsError() {
   }
   return true;
 }
+
+bool AddMissingMacroPound::diagnoseAsError() {
+  emitDiagnostic(diag::macro_expansion_missing_pound, macro->getName())
+    .fixItInsert(getLoc(), "#");
+  return true;
+}

lib/Sema/CSDiagnostics.h

@@ -2854,6 +2854,29 @@ class ConflictingPatternVariables final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
+/// Diagnose situations where we end up type checking a reference to a macro
+/// that was not indicated as a missing # in the source.:
+///
+/// \code
+/// func print(_ value: Any)
+/// @expression macro print<Value...>(_ value: Value...)
+///
+/// func test(e: E) {
+///   print(a, b, c) // missing # to use the macro
+/// }
+/// \endcode
+class AddMissingMacroPound final : public FailureDiagnostic {
+  MacroDecl *macro;
+
+public:
+  AddMissingMacroPound(const Solution &solution, MacroDecl *macro,
+                       ConstraintLocator *locator)
+    : FailureDiagnostic(solution, locator),
+      macro(macro) { }
+
+  bool diagnoseAsError() override;
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSFix.cpp

@@ -2601,3 +2601,15 @@ RenameConflictingPatternVariables::create(ConstraintSystem &cs, Type expectedTy,
   return new (mem)
       RenameConflictingPatternVariables(cs, expectedTy, conflicts, locator);
 }
+
+bool MacroMissingPound::diagnose(const Solution &solution,
+                                                 bool asNote) const {
+  AddMissingMacroPound failure(solution, macro, getLocator());
+  return failure.diagnose(asNote);
+}
+
+MacroMissingPound *
+MacroMissingPound::create(ConstraintSystem &cs, MacroDecl *macro,
+                          ConstraintLocator *locator) {
+  return new (cs.getAllocator()) MacroMissingPound(cs, macro, locator);
+}

lib/Sema/CSGen.cpp

@@ -1482,22 +1482,41 @@ namespace {
                                       TVO_CanBindToLValue | TVO_CanBindToNoEscape);
       ArrayRef<ValueDecl*> decls = expr->getDecls();
       SmallVector<OverloadChoice, 4> choices;
-      
-      for (unsigned i = 0, n = decls.size(); i != n; ++i) {
-        // If the result is invalid, skip it.
-        // FIXME: Note this as invalid, in case we don't find a solution,
-        // so we don't let errors cascade further.
-        if (decls[i]->isInvalid())
-          continue;
+      bool anyMacros = false;
+      auto addChoices = [&](bool skipMacros) {
+        for (unsigned i = 0, n = decls.size(); i != n; ++i) {
+          // If the result is invalid, skip it.
+          // FIXME: Note this as invalid, in case we don't find a solution,
+          // so we don't let errors cascade further.
+          if (decls[i]->isInvalid())
+            continue;
 
-        OverloadChoice choice =
-            OverloadChoice(Type(), decls[i], expr->getFunctionRefKind());
-        choices.push_back(choice);
-      }
+          // If the result is a macro, skip it if we're supposed to.
+          if (skipMacros && isa<MacroDecl>(decls[i])) {
+            anyMacros = true;
+            continue;
+          }
 
-      // If there are no valid overloads, give up.
-      if (choices.empty())
-        return nullptr;
+          OverloadChoice choice =
+              OverloadChoice(Type(), decls[i], expr->getFunctionRefKind());
+          choices.push_back(choice);
+        }
+      };
+
+      addChoices(/*skipMacros=*/true);
+
+      if (choices.empty()) {
+        // If there are no valid overloads, but we ignored some macros, add
+        // the macros. This improves recovery when the user forgot the leading
+        // '#'.
+        if (anyMacros) {
+          addChoices(/*skipMacros=*/false);
+          assert(!choices.empty());
+        } else {
+          // There are no suitable overloads. Just fail.
+          return nullptr;
+        }
+      }
 
       // Record this overload set.
       CS.addOverloadSet(tv, choices, CurDC, locator);

lib/Sema/CSSimplify.cpp

@@ -13809,7 +13809,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::AllowAutoClosurePointerConversion:
   case FixKind::IgnoreKeyPathContextualMismatch:
   case FixKind::NotCompileTimeConst:
-  case FixKind::RenameConflictingPatternVariables: {
+  case FixKind::RenameConflictingPatternVariables:
+  case FixKind::MacroMissingPound: {
     return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
   }
   case FixKind::IgnoreInvalidASTNode: {

lib/Sema/ConstraintLocator.cpp

@@ -597,6 +597,10 @@ bool ConstraintLocator::isForResultBuilderBodyResult() const {
   return isFirstElement<LocatorPathElt::ResultBuilderBodyResult>();
 }
 
+bool ConstraintLocator::isForMacroExpansion() const {
+  return directlyAt<MacroExpansionExpr>();
+}
+
 GenericTypeParamType *ConstraintLocator::getGenericParameter() const {
   // Check whether we have a path that terminates at a generic parameter.
   return isForGenericParameter() ?

lib/Sema/ConstraintSystem.cpp

@@ -3568,6 +3568,14 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
         // e.g. `foo.bar()` or `Foo.bar(&foo)()`, and there is nothing to do.
       }
     }
+
+    // If we have a macro, it can only be used in an expansion.
+    if (auto macro = dyn_cast<MacroDecl>(decl)) {
+      if (!locator->isForMacroExpansion()) {
+        // Record a fix here
+        (void)recordFix(MacroMissingPound::create(*this, macro, locator));
+      }
+    }
   }
 
   // Note that we have resolved this overload.