Check operator declaration binding in the first pass for FuncDecls.

This eliminates the pre-pass we were performing to bind the operator
decls, which was only a pre-pass because we used to bind unresolved
declaration references too early. In the process, fixed some bugs
(e.g., it wasn't checking methods at all) and improved the QoI with
Fix-Its and notes:

t.swift:2:6: error: prefix unary operator missing 'prefix' attribute
func ~~~(x : Float) {}
     ^
     [prefix] 
t.swift:1:17: note: prefix operator found here
operator prefix ~~~ {}
                ^



Swift SVN r7099

include/swift/AST/Diagnostics.def

@@ -995,6 +995,11 @@ ERROR(declared_operator_without_operator_decl,sema_nb,none,
       "operator implementation without matching operator declaration", ())
 ERROR(declared_unary_op_without_attribute,sema_nb,none,
       "unary operator implementation must have a 'prefix' or 'postfix' attribute", ())
+ERROR(unary_op_missing_prepos_attribute,sema_nb,none,
+      "%select{prefix|postfix}0 unary operator missing "
+      "'%select{prefix|postfix}0' attribute", (bool))
+NOTE(unary_operator_declaration_here,sema_nb,none,
+   "%select{prefix|postfix}0 operator found here", (bool))
 ERROR(invalid_arg_count_for_operator,sema_nb,none,
       "operators must have one or two arguments", ())
 

lib/AST/Module.cpp

@@ -238,12 +238,12 @@ Optional<OP_DECL *> lookupOperatorDeclForName(Module *M,
 
   auto *TU = dyn_cast<TranslationUnit>(M);
   if (!TU)
-    return nullptr;
+    return Nothing;
 
   // Look for an operator declaration in the current module.
   auto found = (TU->*OP_MAP).find(Name.get());
   if (found != (TU->*OP_MAP).end())
-    return found->getValue();
+    return found->getValue()? Optional<OP_DECL *>(found->getValue()) : Nothing;
   
   // Look for imported operator decls.
   
@@ -262,7 +262,7 @@ Optional<OP_DECL *> lookupOperatorDeclForName(Module *M,
   if (importedOperators.empty()) {
     // Cache the mapping so we don't need to troll imports next time.
     (TU->*OP_MAP)[Name.get()] = nullptr;
-    return nullptr;
+    return Nothing;
   }
   if (importedOperators.size() == 1) {
     // Cache the mapping so we don't need to troll imports next time.

lib/Sema/TypeCheckDecl.cpp

@@ -947,6 +947,88 @@ public:
     FE->setType(funcTy);
   }
 
+  /// Bind the given function declaration, which declares an operator, to
+  /// the corresponding operator declaration.
+  void bindFuncDeclToOperator(FuncDecl *FD) {
+    OperatorDecl *op = nullptr;
+    auto &TU = TC.TU;
+    if (FD->isUnaryOperator()) {
+      if (FD->getAttrs().isPrefix()) {
+        if (auto maybeOp = TU.lookupPrefixOperator(FD->getName(), FD->getLoc()))
+          op = *maybeOp;
+        else
+          return;
+      } else if (FD->getAttrs().isPostfix()) {
+        if (auto maybeOp = TU.lookupPostfixOperator(FD->getName(),FD->getLoc()))
+          op = *maybeOp;
+        else
+          return;
+      } else {
+        auto prefixOp = TU.lookupPrefixOperator(FD->getName(), FD->getLoc());
+        auto postfixOp = TU.lookupPostfixOperator(FD->getName(), FD->getLoc());
+
+        // If we found both prefix and postfix, or neither prefix nor postfix,
+        // complain. We can't fix this situation.
+        if (static_cast<bool>(prefixOp) == static_cast<bool>(postfixOp)) {
+          TC.diagnose(FD, diag::declared_unary_op_without_attribute);
+
+          // If we found both, point at them.
+          if (prefixOp) {
+            SourceLoc insertionLoc = FD->getLoc();
+
+            TC.diagnose(*prefixOp, diag::unary_operator_declaration_here,false)
+              .fixItInsert(insertionLoc, "[prefix] ");
+            TC.diagnose(*postfixOp, diag::unary_operator_declaration_here, true)
+              .fixItInsert(insertionLoc, "[postfix] ");
+          } else {
+            // FIXME: Introduce a Fix-It that adds the operator declaration?
+          }
+
+          // FIXME: Errors could cascade here, because name lookup for this
+          // operator won't find this declaration.
+          return;
+        }
+
+        // We found only one operator declaration, so we know whether this
+        // should be a prefix or a postfix operator.
+
+        // Fix the AST and determine the insertion text.
+        SourceLoc insertionLoc = FD->getLoc();
+        const char *insertionText;
+        if (postfixOp) {
+          insertionText = "[postfix] ";
+          op = *postfixOp;
+          FD->getMutableAttrs().ExplicitPostfix = true;
+        } else {
+          insertionText = "[prefix] ";
+          op = *prefixOp;
+          FD->getMutableAttrs().ExplicitPrefix = true;
+        }
+
+        // Emit diagnostic with the Fix-It.
+        TC.diagnose(FD, diag::unary_op_missing_prepos_attribute,
+                    static_cast<bool>(postfixOp))
+          .fixItInsert(insertionLoc, insertionText);
+        TC.diagnose(op, diag::unary_operator_declaration_here,
+                    static_cast<bool>(postfixOp));
+      }
+    } else if (FD->isBinaryOperator()) {
+      if (auto maybeOp = TU.lookupInfixOperator(FD->getName(), FD->getLoc()))
+        op = *maybeOp;
+      else {
+        // FIXME: Add Fix-It introducing an operator declaration?
+        TC.diagnose(FD, diag::declared_operator_without_operator_decl);
+        return;
+      }
+    } else {
+      TC.diagnose(FD, diag::invalid_arg_count_for_operator);
+      return;
+    }
+
+    assert(op && "Should have computed operator above");
+    FD->setOperatorDecl(op);
+  }
+
   void visitFuncDecl(FuncDecl *FD) {
     if (!IsFirstPass) {
       if (auto body = FD->getBody())
@@ -957,6 +1039,10 @@ public:
     if (IsSecondPass)
       return;
 
+    // Bind operator functions to the corresponding operator declaration.
+    if (FD->isOperator())
+      bindFuncDeclToOperator(FD);
+
     FuncExpr *body = FD->getBody();
 
     // Before anything else, set up the 'this' argument correctly.

lib/Sema/TypeChecker.cpp

@@ -465,41 +465,6 @@ static void bindExtensionDecl(ExtensionDecl *ED, TypeChecker &TC) {
   }
 }
 
-static void bindFuncDeclToOperator(TypeChecker &TC,
-                                   TranslationUnit *TU,
-                                   FuncDecl *FD) {
-  OperatorDecl *op;
-  if (FD->isUnaryOperator()) {
-    if (FD->getAttrs().isPrefix()) {
-      if (auto maybeOp = TU->lookupPrefixOperator(FD->getName(), FD->getLoc()))
-        op = *maybeOp;
-      else
-        return;
-    } else if (FD->getAttrs().isPostfix()) {
-      if (auto maybeOp = TU->lookupPostfixOperator(FD->getName(), FD->getLoc()))
-        op = *maybeOp;
-      else
-        return;
-    } else {
-      TC.diagnose(FD, diag::declared_unary_op_without_attribute);
-      return;
-    }
-  } else if (FD->isBinaryOperator()) {
-    if (auto maybeOp = TU->lookupInfixOperator(FD->getName(), FD->getLoc()))
-      op = *maybeOp;
-    else
-      return;
-  } else {
-    TC.diagnose(FD, diag::invalid_arg_count_for_operator);
-    return;
-  }
-
-  if (!op)
-    TC.diagnose(FD, diag::declared_operator_without_operator_decl);
-  else
-    FD->setOperatorDecl(op);
-}
-
 /// performTypeChecking - Once parsing and namebinding are complete, these
 /// walks the AST to resolve types and diagnose problems therein.
 ///
@@ -514,21 +479,13 @@ void swift::performTypeChecking(TranslationUnit *TU, unsigned StartElem) {
   TypeChecker TC(*TU);
   auto &FuncExprs = TC.definedFunctions;
 
-  // Resolve extensions and operator declarations.
-  // FIXME: Feels too early to care about operator declarations.
+  // Resolve extensions. This has to occur first during type checking,
+  // because
   for (unsigned i = StartElem, e = TU->Decls.size(); i != e; ++i) {
     if (ExtensionDecl *ED = dyn_cast<ExtensionDecl>(TU->Decls[i])) {
       bindExtensionDecl(ED, TC);
       continue;
     }
-
-    if (FuncDecl *FD = dyn_cast<FuncDecl>(TU->Decls[i])) {
-      // If this is an operator implementation, bind it to an operator decl.
-      if (FD->isOperator())
-        bindFuncDeclToOperator(TC, TU, FD);
-
-      continue;
-    }
   }
 
   // FIXME: Check for cycles in class inheritance here?