Add some basic validation of vars and funcs

Check for:

• Matching decl kinds
• Matching PBD shapes (does every VarDecl on both sides have a counterpart?)
• Matching function effects
• Matching function arity (roughly)

Author: beccadax

include/swift/AST/DiagnosticsSema.def

@@ -8111,5 +8111,35 @@ ERROR(availability_value_generic_type_only_version_newer, none,
 ERROR(invalid_value_for_type_same_type,none,
       "cannot constrain type parameter %0 to be integer %1", (Type, Type))
 
+//===----------------------------------------------------------------------===//
+// MARK: @abi Attribute
+//===----------------------------------------------------------------------===//
+
+ERROR(attr_abi_mismatched_kind,none,
+      "cannot give %kind0 the ABI of a %1",
+      (Decl *, DescriptiveDeclKind))
+
+ERROR(attr_abi_mismatched_arity,none,
+      "cannot give %kind0 the ABI of a %kindonly0 with a different number of "
+      "low-level parameters",
+      (ValueDecl *))
+
+ERROR(attr_abi_mismatched_throws,none,
+      "cannot give %0 the ABI of a %kindonly0 which %select{cannot|can}1 throw",
+      (ValueDecl *, /*abiCanThrow=*/bool))
+
+ERROR(attr_abi_mismatched_async,none,
+      "cannot give %0 the ABI of %select{a non-async|an async}1 %kindonly0",
+      (ValueDecl *, /*abiIsAsync=*/bool))
+
+ERROR(attr_abi_mismatched_pbd_size,none,
+      "cannot give pattern binding the ABI of a binding with "
+      "%select{more|fewer}0 patterns",
+      (/*abiHasExtra=*/bool))
+
+ERROR(attr_abi_mismatched_var,none,
+      "no match for %select{%kind0 in the ABI|ABI %kind0}1",
+      (ValueDecl *, /*isABI=*/bool))
+
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

lib/Sema/TypeCheckAttr.cpp

@@ -188,15 +188,85 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
   IGNORED_ATTR(PreInverseGenerics)
 #undef IGNORED_ATTR
 
+private:
+  static unsigned getABIArity(AbstractFunctionDecl *afd) {
+    unsigned arity = afd->getParameters()->size();
+    arity += afd->getGenericSignature().getGenericParams().size();
+    if (afd->hasImplicitSelfDecl())
+      arity += 1;
+    return arity;
+  }
+
+  void checkABIAttrPBD(PatternBindingDecl *APBD, VarDecl *VD) {
+    auto PBD = VD->getParentPatternBinding();
+
+    // To make sure we only diagnose this stuff once, check that VD is the first
+    // anchoring variable in the PBD.
+    bool isFirstAnchor = false;
+    for (auto i : range(PBD->getNumPatternEntries())) {
+      auto anchorVD = PBD->getAnchoringVarDecl(i);
+      if (anchorVD) {
+        isFirstAnchor = (anchorVD == VD);
+        break;
+      }
+    }
+
+    if (!isFirstAnchor)
+      return;
+
+    // Check that the PBDs have the same number of patterns.
+    if (PBD->getNumPatternEntries() < APBD->getNumPatternEntries()) {
+      diagnose(APBD->getPattern(PBD->getNumPatternEntries())->getLoc(),
+               diag::attr_abi_mismatched_pbd_size, /*abiHasExtra=*/false);
+      return;
+    }
+    if (PBD->getNumPatternEntries() > APBD->getNumPatternEntries()) {
+      diagnose(PBD->getPattern(APBD->getNumPatternEntries())->getLoc(),
+               diag::attr_abi_mismatched_pbd_size, /*abiHasExtra=*/true);
+      return;
+    }
+
+    // Check that each pattern has the same number of variables.
+    for (auto i : range(PBD->getNumPatternEntries())) {
+      SmallVector<VarDecl *, 8> VDs;
+      SmallVector<VarDecl *, 8> AVDs;
+
+      PBD->getPattern(i)->collectVariables(VDs);
+      APBD->getPattern(i)->collectVariables(AVDs);
+
+      if (VDs.size() < AVDs.size()) {
+        for (auto AVD : drop_begin(AVDs, VDs.size())) {
+          AVD->diagnose(diag::attr_abi_mismatched_var,
+                        AVD, /*isABI=*/true);
+        }
+      }
+      else if (VDs.size() > AVDs.size()) {
+        for (auto VD : drop_begin(VDs, AVDs.size())) {
+          VD->diagnose(diag::attr_abi_mismatched_var,
+                       VD, /*isABI=*/false);
+        }
+      }
+    }
+  }
+
+public:
   void visitABIAttr(ABIAttr *attr) {
     // Inverse ABI attrs are an implementation detail and don't need checking.
     if (attr->isInverse())
       return;
 
     Decl *AD = attr->abiDecl;
     if (isa<VarDecl>(D) && isa<PatternBindingDecl>(AD)) {
-      AD = cast<PatternBindingDecl>(AD)
-               ->getVarAtSimilarStructuralPosition(cast<VarDecl>(D));
+      auto VD = cast<VarDecl>(D);
+      auto APBD = cast<PatternBindingDecl>(AD);
+
+      // Diagnose dissimilar PBD structures.
+      checkABIAttrPBD(APBD, VD);
+
+      // Do the rest of this checking on the corresponding VarDecl, not the
+      // PBD that's actually in the attribute. Note that `AD` will become null
+      // if they're too dissimilar to match up.
+      AD = APBD->getVarAtSimilarStructuralPosition(VD);
     }
     // TODO: EnumElementDecl?
 
@@ -208,7 +278,52 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
     if (AD->isInvalid())
       return;
 
+    // Do the declarations have the same kind, broadly speaking? Many kinds have
+    // special mangling behavior (e.g. inits vs normal funcs) that make it
+    // unrealistic to treat one kind as though it were another.
+    if (D->getKind() != AD->getKind()) {
+      // FIXME: DescriptiveDeclKind is overly specific; we really just want to
+      //        say that e.g. a `func` can't have the ABI of a `var`.
+      diagnoseAndRemoveAttr(attr, diag::attr_abi_mismatched_kind,
+                            D, AD->getDescriptiveKind());
+      return;
+    }
+
+    if (isa<AbstractFunctionDecl>(D)) {
+      auto AFD = cast<AbstractFunctionDecl>(D);
+      auto AAFD = cast<AbstractFunctionDecl>(AD);
+
+      // FIXME: How much should we diagnose in IRGen for more precise ABI info?
+
+      // Do the declarations have roughly the same number of parameters? We'll
+      // allow some fuzziness for what these parameters *are*, since there isn't
+      // always an ABI difference between e.g. a free function with N parameters
+      // and an instance method with N-1 parameters (plus an implicit `self`).
+      if (getABIArity(AFD) != getABIArity(AAFD)) {
+        diagnoseAndRemoveAttr(attr, diag::attr_abi_mismatched_arity,
+                              AFD);
+      }
+
+      // Do the declarations match in throwing behavior? We don't care about
+      // `throws` vs. `rethrows` here, just whether callers will account for an
+      // error return.
+      // FIXME: Typed throws?
+      if (AFD->hasThrows() != AAFD->hasThrows()) {
+        diagnoseAndRemoveAttr(attr, diag::attr_abi_mismatched_throws,
+                              AFD, /*abiCanThrow=*/AAFD->hasThrows());
+      }
+
+      // Do the declarations match in async-ness?
+      if (AFD->hasAsync() != AAFD->hasAsync()) {
+        diagnoseAndRemoveAttr(attr, diag::attr_abi_mismatched_async,
+                              AFD, /*abiHasAsync=*/AAFD->hasAsync());
+      }
+    }
+
     // TODO: Validate more
+    // FIXME: The list of properties that have to match is practically endless
+    // and will grow as new features are added to the compiler. We might want to
+    // write an AttributeVisitor just to help us catch omissions over time.
   }
 
   void visitAlignmentAttr(AlignmentAttr *attr) {

test/attr/attr_abi.swift

@@ -13,10 +13,10 @@ func funcForFunc() {}
 @abi(var varForVar_abi: Int)
 var varForVar: Int = 0
 
-@abi(func funcForVar_abi())
+@abi(func funcForVar_abi()) // expected-error {{cannot give var 'funcForVar' the ABI of a global function}}
 var funcForVar: Int = 0
 
-@abi(var varForFunc_abi: Int)
+@abi(var varForFunc_abi: Int) // expected-error {{cannot give global function 'varForFunc()' the ABI of a pattern binding}}
 func varForFunc() {}
 
 //
@@ -26,52 +26,52 @@ func varForFunc() {}
 @abi(func param00_generic00() -> Int)
 func param00_generic00() -> Int { fatalError() }
 
-@abi(func param10_generic00(_: Int) -> Int)
+@abi(func param10_generic00(_: Int) -> Int) // expected-error {{cannot give global function 'param10_generic00()' the ABI of a global function with a different number of low-level parameters}}
 func param10_generic00() -> Int { fatalError() }
 
-@abi(func param01_generic00() -> Int)
+@abi(func param01_generic00() -> Int) // expected-error {{cannot give global function 'param01_generic00' the ABI of a global function with a different number of low-level parameters}}
 func param01_generic00(_: Int) -> Int { fatalError() }
 
 @abi(func param11_generic00(_: Int) -> Int)
 func param11_generic00(_: Int) -> Int { fatalError() }
 
 
 
-@abi(func param00_generic10<T>() -> T)
+@abi(func param00_generic10<T>() -> T) // expected-error {{cannot give global function 'param00_generic10()' the ABI of a global function with a different number of low-level parameters}}
 func param00_generic10() -> Int { fatalError() }
 
-@abi(func param10_generic10<T>(_: Int) -> T)
+@abi(func param10_generic10<T>(_: Int) -> T) // expected-error {{cannot give global function 'param10_generic10()' the ABI of a global function with a different number of low-level parameters}}
 func param10_generic10() -> Int { fatalError() }
 
 @abi(func param01_generic10<T>() -> T)
 func param01_generic10(_: Int) -> Int { fatalError() }
 
-@abi(func param11_generic10<T>(_: Int) -> T)
+@abi(func param11_generic10<T>(_: Int) -> T) // expected-error {{cannot give global function 'param11_generic10' the ABI of a global function with a different number of low-level parameters}}
 func param11_generic10(_: Int) -> Int { fatalError() }
 
 
 
-@abi(func param00_generic01() -> Int)
+@abi(func param00_generic01() -> Int) // expected-error {{cannot give global function 'param00_generic01()' the ABI of a global function with a different number of low-level parameters}}
 func param00_generic01<T>() -> T { fatalError() }
 
 @abi(func param10_generic01(_: Int) -> Int)
 func param10_generic01<T>() -> T { fatalError() }
 
-@abi(func param01_generic01() -> Int)
+@abi(func param01_generic01() -> Int) // expected-error {{cannot give global function 'param01_generic01' the ABI of a global function with a different number of low-level parameters}}
 func param01_generic01<T>(_: Int) -> T { fatalError() }
 
-@abi(func param11_generic01(_: Int) -> Int)
+@abi(func param11_generic01(_: Int) -> Int) // expected-error {{cannot give global function 'param11_generic01' the ABI of a global function with a different number of low-level parameters}}
 func param11_generic01<T>(_: Int) -> T { fatalError() }
 
 
 
 @abi(func param00_generic11<T>() -> T)
 func param00_generic11<T>() -> T { fatalError() }
 
-@abi(func param10_generic11<T>(_: Int) -> T)
+@abi(func param10_generic11<T>(_: Int) -> T) // expected-error {{cannot give global function 'param10_generic11()' the ABI of a global function with a different number of low-level parameters}}
 func param10_generic11<T>() -> T { fatalError() }
 
-@abi(func param01_generic11<T>() -> T)
+@abi(func param01_generic11<T>() -> T) // expected-error {{cannot give global function 'param01_generic11' the ABI of a global function with a different number of low-level parameters}}
 func param01_generic11<T>(_: Int) -> T { fatalError() }
 
 @abi(func param11_generic11<T>(_: Int) -> T)
@@ -84,13 +84,13 @@ func param11_generic11<T>(_: Int) -> T { fatalError() }
 @abi(func throws00(_: () throws -> Void))
 func throws00(_: () throws -> Void) {}
 
-@abi(func throws10(_: () throws -> Void) throws)
+@abi(func throws10(_: () throws -> Void) throws) // expected-error {{cannot give 'throws10' the ABI of a global function which can throw}}
 func throws10(_: () throws -> Void) {}
 
-@abi(func throws20(_: () throws -> Void) rethrows)
+@abi(func throws20(_: () throws -> Void) rethrows) // expected-error {{cannot give 'throws20' the ABI of a global function which can throw}}
 func throws20(_: () throws -> Void) {}
 
-@abi(func throws01(_: () throws -> Void))
+@abi(func throws01(_: () throws -> Void)) // expected-error {{cannot give 'throws01' the ABI of a global function which cannot throw}}
 func throws01(_: () throws -> Void) throws {}
 
 @abi(func throws11(_: () throws -> Void) throws)
@@ -99,7 +99,7 @@ func throws11(_: () throws -> Void) throws {}
 @abi(func throws21(_: () throws -> Void) rethrows)
 func throws21(_: () throws -> Void) throws {}
 
-@abi(func throws02(_: () throws -> Void))
+@abi(func throws02(_: () throws -> Void)) // expected-error {{cannot give 'throws02' the ABI of a global function which cannot throw}}
 func throws02(_: () throws -> Void) rethrows {}
 
 @abi(func throws12(_: () throws -> Void) throws)
@@ -115,10 +115,10 @@ func throws22(_: () throws -> Void) rethrows {}
 @abi(func async00())
 func async00() {}
 
-@abi(func async10() async)
+@abi(func async10() async) // expected-error {{cannot give 'async10()' the ABI of an async global function}}
 func async10() {}
 
-@abi(func async01())
+@abi(func async01()) // expected-error {{cannot give 'async01()' the ABI of a non-async global function}}
 func async01() async {}
 
 @abi(func async11() async)
@@ -128,17 +128,17 @@ func async11() async {}
 // PBD shape checking
 //
 
-@abi(var x1, y1: Int)
+@abi(var x1, y1: Int) // expected-error {{cannot give pattern binding the ABI of a binding with more patterns}}
 var x1: Int = 0
 
 @abi(var x2: Int)
-var x2 = 0, y2: Int = 0
+var x2 = 0, y2: Int = 0 // expected-error {{cannot give pattern binding the ABI of a binding with fewer patterns}}
 
-@abi(var (x3, y3): (Int, Int), (a3, b3): (Int, Int))
+@abi(var (x3, y3): (Int, Int), (a3, b3): (Int, Int)) // expected-error {{no match for ABI var 'b3'}}
 var (x3, y3): (Int, Int) = (0, 0), a3: Int = 0
 
 @abi(var (x4, y4): (Int, Int), a4: Int)
-var (x4, y4): (Int, Int) = (0, 0), (a4, b4): (Int, Int) = (0, 0)
+var (x4, y4): (Int, Int) = (0, 0), (a4, b4): (Int, Int) = (0, 0) // expected-error {{no match for var 'b4' in the ABI}}
 
 //
 // Conflict diagnostics