Merge pull request #64422 from Rajveer100/branch-for-issue-63993

Fix Issue #63993 by improving out-of-place binding diagnostic and reflecting 'var' or 'let' binding pattern

Author: hamishknight

include/swift/AST/DiagnosticEngine.h

@@ -46,7 +46,7 @@ namespace swift {
   class ValueDecl;
   class SourceFile;
 
-  enum class PatternKind : uint8_t;
+  enum class DescriptivePatternKind : uint8_t;
   enum class SelfAccessKind : uint8_t;
   enum class ReferenceOwnership : uint8_t;
   enum class StaticSpellingKind : uint8_t;
@@ -113,7 +113,7 @@ namespace swift {
     Type,
     TypeRepr,
     FullyQualifiedType,
-    PatternKind,
+    DescriptivePatternKind,
     SelfAccessKind,
     ReferenceOwnership,
     StaticSpellingKind,
@@ -147,7 +147,7 @@ namespace swift {
       Type TypeVal;
       TypeRepr *TyR;
       FullyQualified<Type> FullyQualifiedTypeVal;
-      PatternKind PatternKindVal;
+      DescriptivePatternKind DescriptivePatternKindVal;
       SelfAccessKind SelfAccessKindVal;
       ReferenceOwnership ReferenceOwnershipVal;
       StaticSpellingKind StaticSpellingKindVal;
@@ -220,8 +220,9 @@ namespace swift {
       }
     }
 
-    DiagnosticArgument(PatternKind K)
-        : Kind(DiagnosticArgumentKind::PatternKind), PatternKindVal(K) {}
+    DiagnosticArgument(DescriptivePatternKind DPK)
+        : Kind(DiagnosticArgumentKind::DescriptivePatternKind),
+          DescriptivePatternKindVal(DPK) {}
 
     DiagnosticArgument(ReferenceOwnership RO)
         : Kind(DiagnosticArgumentKind::ReferenceOwnership),
@@ -324,9 +325,9 @@ namespace swift {
       return FullyQualifiedTypeVal;
     }
 
-    PatternKind getAsPatternKind() const {
-      assert(Kind == DiagnosticArgumentKind::PatternKind);
-      return PatternKindVal;
+    DescriptivePatternKind getAsDescriptivePatternKind() const {
+      assert(Kind == DiagnosticArgumentKind::DescriptivePatternKind);
+      return DescriptivePatternKindVal;
     }
 
     ReferenceOwnership getAsReferenceOwnership() const {

include/swift/AST/DiagnosticsSema.def

@@ -1212,7 +1212,7 @@ ERROR(optional_chain_isnt_chaining,none,
       "'?' must be followed by a call, member lookup, or subscript",
       ())
 ERROR(pattern_in_expr,none,
-      "%0 cannot appear in an expression", (PatternKind))
+      "%0 cannot appear in an expression", (DescriptivePatternKind))
 NOTE(note_call_to_operator,none,
      "in call to operator %0", (DeclName))
 NOTE(note_call_to_func,none,

include/swift/AST/Pattern.h

@@ -47,8 +47,20 @@ enum class PatternKind : uint8_t {
 enum : unsigned { NumPatternKindBits =
   countBitsUsed(static_cast<unsigned>(PatternKind::Last_Pattern)) };
 
-/// Diagnostic printing of PatternKinds.
-llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, PatternKind kind);
+enum class DescriptivePatternKind : uint8_t {
+  Paren,
+  Tuple,
+  Named,
+  Any,
+  Typed,
+  Is,
+  EnumElement,
+  OptionalSome,
+  Bool,
+  Expr,
+  Var,
+  Let
+};
 
 /// Pattern - Base class for all patterns in Swift.
 class alignas(8) Pattern : public ASTAllocated<Pattern> {
@@ -105,13 +117,20 @@ class alignas(8) Pattern : public ASTAllocated<Pattern> {
 public:
   PatternKind getKind() const { return PatternKind(Bits.Pattern.Kind); }
 
+  /// Retrieve the descriptive pattern kind for this pattern.
+  DescriptivePatternKind getDescriptiveKind() const;
+
   /// Retrieve the name of the given pattern kind.
   ///
   /// This name should only be used for debugging dumps and other
   /// developer aids, and should never be part of a diagnostic or exposed
   /// to the user of the compiler in any way.
   static StringRef getKindName(PatternKind K);
 
+  /// Produce a name for the given descriptive pattern kind, which
+  /// is suitable for use in diagnostics.
+  static StringRef getDescriptivePatternKindName(DescriptivePatternKind K);
+
   /// A pattern is implicit if it is compiler-generated and there
   /// exists no source code for it.
   bool isImplicit() const { return Bits.Pattern.isImplicit; }

lib/AST/DiagnosticEngine.cpp

@@ -747,9 +747,11 @@ static void formatDiagnosticArgument(StringRef Modifier,
         << FormatOpts.ClosingQuotationMark;
     break;
 
-  case DiagnosticArgumentKind::PatternKind:
-    assert(Modifier.empty() && "Improper modifier for PatternKind argument");
-    Out << Arg.getAsPatternKind();
+  case DiagnosticArgumentKind::DescriptivePatternKind:
+    assert(Modifier.empty() &&
+           "Improper modifier for DescriptivePatternKind argument");
+    Out << Pattern::getDescriptivePatternKindName(
+        Arg.getAsDescriptivePatternKind());
     break;
 
   case DiagnosticArgumentKind::SelfAccessKind:

lib/AST/Pattern.cpp

@@ -32,33 +32,29 @@ using namespace swift;
                 "Patterns are BumpPtrAllocated; the d'tor is never called");
 #include "swift/AST/PatternNodes.def"
 
-/// Diagnostic printing of PatternKinds.
-llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &OS, PatternKind kind) {
-  switch (kind) {
-  case PatternKind::Paren:
-    return OS << "parenthesized pattern";
-  case PatternKind::Tuple:
-    return OS << "tuple pattern";
-  case PatternKind::Named:
-    return OS << "pattern variable binding";
-  case PatternKind::Any:
-    return OS << "'_' pattern";
-  case PatternKind::Typed:
-    return OS << "pattern type annotation";
-  case PatternKind::Is:
-    return OS << "prefix 'is' pattern";
-  case PatternKind::Expr:
-    return OS << "expression pattern";
+DescriptivePatternKind Pattern::getDescriptiveKind() const {
+#define TRIVIAL_PATTERN_KIND(Kind)                                             \
+  case PatternKind::Kind:                                                      \
+    return DescriptivePatternKind::Kind
+
+  switch (getKind()) {
+    TRIVIAL_PATTERN_KIND(Paren);
+    TRIVIAL_PATTERN_KIND(Tuple);
+    TRIVIAL_PATTERN_KIND(Named);
+    TRIVIAL_PATTERN_KIND(Any);
+    TRIVIAL_PATTERN_KIND(Typed);
+    TRIVIAL_PATTERN_KIND(Is);
+    TRIVIAL_PATTERN_KIND(EnumElement);
+    TRIVIAL_PATTERN_KIND(OptionalSome);
+    TRIVIAL_PATTERN_KIND(Bool);
+    TRIVIAL_PATTERN_KIND(Expr);
+
   case PatternKind::Binding:
-    return OS << "'var' binding pattern";
-  case PatternKind::EnumElement:
-    return OS << "enum case matching pattern";
-  case PatternKind::OptionalSome:
-    return OS << "optional .Some matching pattern";
-  case PatternKind::Bool:
-    return OS << "bool matching pattern";
+    return cast<BindingPattern>(this)->isLet() ? DescriptivePatternKind::Let
+                                               : DescriptivePatternKind::Var;
   }
-  llvm_unreachable("bad PatternKind");
+#undef TRIVIAL_PATTERN_KIND
+  llvm_unreachable("bad DescriptivePatternKind");
 }
 
 StringRef Pattern::getKindName(PatternKind K) {
@@ -69,6 +65,28 @@ StringRef Pattern::getKindName(PatternKind K) {
   llvm_unreachable("bad PatternKind");
 }
 
+StringRef Pattern::getDescriptivePatternKindName(DescriptivePatternKind K) {
+#define ENTRY(Kind, String)                                                    \
+  case DescriptivePatternKind::Kind:                                           \
+    return String
+  switch (K) {
+    ENTRY(Paren, "parenthesized pattern");
+    ENTRY(Tuple, "tuple pattern");
+    ENTRY(Named, "pattern variable binding");
+    ENTRY(Any, "'_' pattern");
+    ENTRY(Typed, "pattern type annotation");
+    ENTRY(Is, "prefix 'is' pattern");
+    ENTRY(EnumElement, "enum case matching pattern");
+    ENTRY(OptionalSome, "optional pattern");
+    ENTRY(Bool, "bool matching pattern");
+    ENTRY(Expr, "expression pattern");
+    ENTRY(Var, "'var' binding pattern");
+    ENTRY(Let, "'let' binding pattern");
+  }
+#undef ENTRY
+  llvm_unreachable("bad DescriptivePatternKind");
+}
+
 // Metaprogram to verify that every concrete class implements
 // a 'static bool classof(const Pattern*)'.
 template <bool fn(const Pattern*)> struct CheckClassOfPattern {

lib/Sema/CSApply.cpp

@@ -4558,7 +4558,7 @@ namespace {
           && !cs.getType(simplified)->is<UnresolvedType>()) {
         auto &de = cs.getASTContext().Diags;
         de.diagnose(simplified->getLoc(), diag::pattern_in_expr,
-                    expr->getSubPattern()->getKind());
+                    expr->getSubPattern()->getDescriptiveKind());
       }
       return simplified;
     }

test/Sema/redeclaration-checking.swift

@@ -123,11 +123,10 @@ let issue63750 = {
     // expected-note@-2 {{'x' previously declared here}}
     ()
   }
-  // FIXME: We should really say 'let' instead of 'var' in "'var' binding pattern cannot appear in an expression"
-  // https://github.com/apple/swift/issues/63993
+  
   func bar(_ x: Int) -> Int { x }
   if case (bar(let x), let x) = (0,0) {}
-  // expected-error@-1 {{'var' binding pattern cannot appear in an expression}}
+  // expected-error@-1 {{'let' binding pattern cannot appear in an expression}}
   // expected-error@-2 {{invalid redeclaration of 'x'}}
   // expected-note@-3 {{'x' previously declared here}}
 }
@@ -149,7 +148,7 @@ func issue63750fn() {
   }
   func bar(_ x: Int) -> Int { x }
   if case (bar(let x), let x) = (0,0) {}
-  // expected-error@-1 {{'var' binding pattern cannot appear in an expression}}
+  // expected-error@-1 {{'let' binding pattern cannot appear in an expression}}
   // expected-error@-2 {{invalid redeclaration of 'x'}}
   // expected-note@-3 {{'x' previously declared here}}
 }