[PreCheck] Filter out macro declarations from result set

Move logic from `ConstraintGenerator::visitOverloadedDeclRefExpr`
to pre-check to avoid including macro declarations referenced
without `#`. This means that pre-checking would synthesize
`TypeExpr` in situations when there is a type reference that
is shadowed by a stdlib macro.

Resolves: #67815
Resolves: rdar://114796811

Author: xedin

lib/Sema/CSGen.cpp

@@ -1658,40 +1658,22 @@ namespace {
                                       TVO_CanBindToLValue | TVO_CanBindToNoEscape);
       ArrayRef<ValueDecl*> decls = expr->getDecls();
       SmallVector<OverloadChoice, 4> choices;
-      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;
-
-          // If the result is a macro, skip it if we're supposed to.
-          if (skipMacros && isa<MacroDecl>(decls[i])) {
-            anyMacros = true;
-            continue;
-          }
 
-          OverloadChoice choice =
-              OverloadChoice(Type(), decls[i], expr->getFunctionRefKind());
-          choices.push_back(choice);
-        }
-      };
+      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;
 
-      addChoices(/*skipMacros=*/true);
+        OverloadChoice choice =
+            OverloadChoice(Type(), decls[i], expr->getFunctionRefKind());
+        choices.push_back(choice);
+      }
 
       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;
-        }
+        // There are no suitable overloads. Just fail.
+        return nullptr;
       }
 
       // Record this overload set.

lib/Sema/PreCheckExpr.cpp

@@ -650,16 +650,12 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
 
   // FIXME: Need to refactor the way we build an AST node from a lookup result!
 
-  // If we have an unambiguous reference to a type decl, form a TypeExpr.
-  if (Lookup.size() == 1 && UDRE->getRefKind() == DeclRefKind::Ordinary &&
-      isa<TypeDecl>(Lookup[0].getValueDecl())) {
-    auto *D = cast<TypeDecl>(Lookup[0].getValueDecl());
+  auto buildTypeExpr = [&](TypeDecl *D) -> Expr * {
     // FIXME: This is odd.
     if (isa<ModuleDecl>(D)) {
-      return new (Context) DeclRefExpr(D, UDRE->getNameLoc(),
-                                       /*Implicit=*/false,
-                                       AccessSemantics::Ordinary,
-                                       D->getInterfaceType());
+      return new (Context) DeclRefExpr(
+          D, UDRE->getNameLoc(),
+          /*Implicit=*/false, AccessSemantics::Ordinary, D->getInterfaceType());
     }
 
     auto *LookupDC = Lookup[0].getDeclContext();
@@ -668,12 +664,19 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
           UDRE->getNameLoc(), D, LookupDC,
           // It might happen that LookupDC is null if this is checking
           // synthesized code, in that case, don't map the type into context,
-          // but return as is -- the synthesis should ensure the type is correct.
+          // but return as is -- the synthesis should ensure the type is
+          // correct.
           LookupDC ? LookupDC->mapTypeIntoContext(D->getInterfaceType())
                    : D->getInterfaceType());
     } else {
       return TypeExpr::createForDecl(UDRE->getNameLoc(), D, LookupDC);
     }
+  };
+
+  // If we have an unambiguous reference to a type decl, form a TypeExpr.
+  if (Lookup.size() == 1 && UDRE->getRefKind() == DeclRefKind::Ordinary &&
+      isa<TypeDecl>(Lookup[0].getValueDecl())) {
+    return buildTypeExpr(cast<TypeDecl>(Lookup[0].getValueDecl()));
   }
 
   if (AllDeclRefs) {
@@ -710,6 +713,24 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
       });
     }
 
+    // Filter out macro declarations without `#` if there are valid
+    // non-macro results.
+    if (llvm::any_of(ResultValues,
+                     [](const ValueDecl *D) { return !isa<MacroDecl>(D); })) {
+      ResultValues.erase(
+          llvm::remove_if(ResultValues,
+                          [](const ValueDecl *D) { return isa<MacroDecl>(D); }),
+          ResultValues.end());
+
+      // If there is only one type reference in results, let's handle
+      // this in a special way.
+      if (ResultValues.size() == 1 &&
+          UDRE->getRefKind() == DeclRefKind::Ordinary &&
+          isa<TypeDecl>(ResultValues.front())) {
+        return buildTypeExpr(cast<TypeDecl>(ResultValues.front()));
+      }
+    }
+
     return buildRefExpr(ResultValues, DC, UDRE->getNameLoc(),
                         UDRE->isImplicit(), UDRE->getFunctionRefKind());
   }

test/Constraints/observable_macro_shadowing.swift

@@ -0,0 +1,38 @@
+// RUN: %empty-directory(%t/src)
+// RUN: %empty-directory(%t/sdk)
+// RUN: split-file %s %t/src
+
+// RUN: %target-swift-frontend -emit-module %t/src/Test.swift \
+// RUN:   -module-name Test -swift-version 5 -enable-library-evolution \
+// RUN:   -emit-module-path %t/Test.swiftmodule
+
+// RUN: %target-swift-frontend -typecheck %t/src/main.swift \
+// RUN:   -module-name main -I %t -verify
+
+// REQUIRES: swift_swift_parser
+// REQUIRES: observation
+
+//--- Test.swift
+
+public protocol ObservableConvertibleType {
+  associatedtype Element
+}
+
+public protocol ObservableType : ObservableConvertibleType {}
+
+public class Observable<Element> : ObservableType {
+}
+
+extension ObservableType {
+  public static func empty() -> Observable<Element> { fatalError() }
+}
+
+//--- main.swift
+import Test
+import Observation
+
+extension Observable {
+  func test() -> Observable<Bool> {
+    return Observable<Bool>.empty()
+  }
+}

test/Macros/macro_and_typealias.swift

@@ -9,13 +9,11 @@
 @freestanding(expression) public macro ConcretePrint(_ value: Any) = #externalMacro(module: "MacroDefinition", type: "PrintMacro")
 @freestanding(expression) public macro MultiPrint(_ value: Any) = #externalMacro(module: "MacroDefinition", type: "PrintMacro")
 
-public struct Printer<Value> {
+public struct Printer<Value> { // expected-note {{generic type 'Printer' declared here}}
   init(_: (Value) -> Void) {}
 }
 
-public struct MultiPrinter<T, U> {
-  // expected-note@-1 {{'T' declared as parameter to type 'MultiPrinter'}}
-  // expected-note@-2 {{'U' declared as parameter to type 'MultiPrinter'}}
+public struct MultiPrinter<T, U> { // expected-note {{generic type 'MultiPrinter' declared here}}
 }
 
 typealias Print = Printer
@@ -34,22 +32,20 @@ struct Test {
     }
 
     let _ = Print<Object, Int> {
-      // expected-error@-1 {{generic type 'Print' specialized with too many type parameters (got 2, but expected 1)}}
+      // expected-error@-1 {{generic type 'Printer' specialized with too many type parameters (got 2, but expected 1)}}
     }
 
     let _ = OtherPrint<Object> { // Ok
       compute(root: $0, \.prop)
     }
 
     let _ = ConcretePrint<Object> { // expected-error {{cannot specialize non-generic type 'ConcretePrint' (aka 'Printer<Any>')}}
-      compute(root: $0, \.prop) // expected-error {{value of type 'Any' has no member 'prop'}}
-      // expected-note@-1 {{cast 'Any' to 'AnyObject' or use 'as!' to force downcast to a more specific type to access members}}
+      // expected-error@-1 {{cannot infer type of closure parameter '$0' without a type annotation}}
+      compute(root: $0, \.prop)
     }
 
     let _ = MultiPrint<Int>()
-    // expected-error@-1 {{generic type 'MultiPrint' specialized with too few type parameters (got 1, but expected 2)}}
-    // expected-error@-2 {{generic parameter 'T' could not be inferred}}
-    // expected-error@-3 {{generic parameter 'U' could not be inferred}}
+    // expected-error@-1 {{generic type 'MultiPrinter' specialized with too few type parameters (got 1, but expected 2)}}
   }
 
   func compute<R, V>(root: R, _: KeyPath<R, V>) {}