[CSDiagnostics] Allow non-ephemeral diag to refer to argument… (#28084)

[CSDiagnostics] Allow non-ephemeral diag to refer to argument labels

Author: hamishknight

include/swift/AST/DiagnosticsSema.def

@@ -401,18 +401,18 @@ ERROR(cannot_convert_argument_value_generic,none,
 
 // @_nonEphemeral conversion diagnostics
 ERROR(cannot_pass_type_to_non_ephemeral,none,
-      "cannot pass %0 to parameter; argument #%1 must be a pointer that "
-      "outlives the call%select{| to %3}2", (Type, unsigned, bool, DeclName))
+      "cannot pass %0 to parameter; argument %1 must be a pointer that "
+      "outlives the call%select{| to %3}2", (Type, StringRef, bool, DeclName))
 WARNING(cannot_pass_type_to_non_ephemeral_warning,none,
-        "passing %0 to parameter, but argument #%1 should be a pointer that "
-        "outlives the call%select{| to %3}2", (Type, unsigned, bool, DeclName))
+        "passing %0 to parameter, but argument %1 should be a pointer that "
+        "outlives the call%select{| to %3}2", (Type, StringRef, bool, DeclName))
 ERROR(cannot_use_inout_non_ephemeral,none,
-      "cannot use inout expression here; argument #%0 must be a pointer that "
-      "outlives the call%select{| to %2}1", (unsigned, bool, DeclName))
+      "cannot use inout expression here; argument %0 must be a pointer that "
+      "outlives the call%select{| to %2}1", (StringRef, bool, DeclName))
 WARNING(cannot_use_inout_non_ephemeral_warning,none,
-        "inout expression creates a temporary pointer, but argument #%0 should "
+        "inout expression creates a temporary pointer, but argument %0 should "
         "be a pointer that outlives the call%select{| to %2}1",
-        (unsigned, bool, DeclName))
+        (StringRef, bool, DeclName))
 ERROR(cannot_construct_dangling_pointer,none,
       "initialization of %0 results in a dangling %select{|buffer }1pointer",
       (Type, unsigned))

lib/Sema/CSDiagnostics.cpp

@@ -286,8 +286,8 @@ FailureDiagnostic::getFunctionArgApplyInfo(ConstraintLocator *locator) const {
   auto argIdx = applyArgElt->getArgIdx();
   auto paramIdx = applyArgElt->getParamIdx();
 
-  return FunctionArgApplyInfo(argExpr, argIdx, getType(argExpr), paramIdx,
-                              fnInterfaceType, fnType, callee);
+  return FunctionArgApplyInfo(cs, argExpr, argIdx, getType(argExpr),
+                              paramIdx, fnInterfaceType, fnType, callee);
 }
 
 Type FailureDiagnostic::restoreGenericParameters(
@@ -5680,21 +5680,24 @@ bool NonEphemeralConversionFailure::diagnoseAsError() {
     return true;
 
   // Otherwise, emit a more general diagnostic.
+  SmallString<8> scratch;
+  auto argDesc = getArgDescription(scratch);
+
   auto *argExpr = getArgExpr();
   if (isa<InOutExpr>(argExpr)) {
     auto diagID = DowngradeToWarning
                       ? diag::cannot_use_inout_non_ephemeral_warning
                       : diag::cannot_use_inout_non_ephemeral;
 
-    emitDiagnostic(argExpr->getLoc(), diagID, getArgPosition(), getCallee(),
+    emitDiagnostic(argExpr->getLoc(), diagID, argDesc, getCallee(),
                    getCalleeFullName())
         .highlight(argExpr->getSourceRange());
   } else {
     auto diagID = DowngradeToWarning
                       ? diag::cannot_pass_type_to_non_ephemeral_warning
                       : diag::cannot_pass_type_to_non_ephemeral;
 
-    emitDiagnostic(argExpr->getLoc(), diagID, getArgType(), getArgPosition(),
+    emitDiagnostic(argExpr->getLoc(), diagID, getArgType(), argDesc,
                    getCallee(), getCalleeFullName())
         .highlight(argExpr->getSourceRange());
   }

lib/Sema/CSDiagnostics.h

@@ -212,6 +212,7 @@ class FailureDiagnostic {
 /// Provides information about the application of a function argument to a
 /// parameter.
 class FunctionArgApplyInfo {
+  ConstraintSystem &CS;
   Expr *ArgExpr;
   unsigned ArgIdx;
   Type ArgType;
@@ -223,11 +224,12 @@ class FunctionArgApplyInfo {
   const ValueDecl *Callee;
 
 public:
-  FunctionArgApplyInfo(Expr *argExpr, unsigned argIdx, Type argType,
-                       unsigned paramIdx, Type fnInterfaceType,
+  FunctionArgApplyInfo(ConstraintSystem &cs, Expr *argExpr, unsigned argIdx,
+                       Type argType, unsigned paramIdx, Type fnInterfaceType,
                        FunctionType *fnType, const ValueDecl *callee)
-      : ArgExpr(argExpr), ArgIdx(argIdx), ArgType(argType), ParamIdx(paramIdx),
-        FnInterfaceType(fnInterfaceType), FnType(fnType), Callee(callee) {}
+      : CS(cs), ArgExpr(argExpr), ArgIdx(argIdx), ArgType(argType),
+        ParamIdx(paramIdx), FnInterfaceType(fnInterfaceType), FnType(fnType),
+        Callee(callee) {}
 
   /// \returns The argument being applied.
   Expr *getArgExpr() const { return ArgExpr; }
@@ -247,6 +249,45 @@ class FunctionArgApplyInfo {
     return withSpecifier ? ArgType : ArgType->getWithoutSpecifierType();
   }
 
+  /// \returns The label for the argument being applied.
+  Identifier getArgLabel() const {
+    auto *parent = CS.getParentExpr(ArgExpr);
+    if (auto *te = dyn_cast<TupleExpr>(parent))
+      return te->getElementName(ArgIdx);
+
+    assert(isa<ParenExpr>(parent));
+    return Identifier();
+  }
+
+  /// \returns A textual description of the argument suitable for diagnostics.
+  /// For an argument with an unambiguous label, this will the label. Otherwise
+  /// it will be its position in the argument list.
+  StringRef getArgDescription(SmallVectorImpl<char> &scratch) const {
+    llvm::raw_svector_ostream stream(scratch);
+
+    // Use the argument label only if it's unique within the argument list.
+    auto argLabel = getArgLabel();
+    auto useArgLabel = [&]() -> bool {
+      if (argLabel.empty())
+        return false;
+
+      if (auto *te = dyn_cast<TupleExpr>(CS.getParentExpr(ArgExpr)))
+        return llvm::count(te->getElementNames(), argLabel) == 1;
+
+      return false;
+    };
+
+    if (useArgLabel()) {
+      stream << "'";
+      stream << argLabel;
+      stream << "'";
+    } else {
+      stream << "#";
+      stream << getArgPosition();
+    }
+    return StringRef(scratch.data(), scratch.size());
+  }
+
   /// \returns The interface type for the function being applied. Note that this
   /// may not a function type, for example it could be a generic parameter.
   Type getFnInterfaceType() const { return FnInterfaceType; }
@@ -1847,6 +1888,13 @@ class ArgumentMismatchFailure : public ContextualFailure {
     return Info->getArgType(withSpecifier);
   }
 
+  /// \returns A textual description of the argument suitable for diagnostics.
+  /// For an argument with an unambiguous label, this will the label. Otherwise
+  /// it will be its position in the argument list.
+  StringRef getArgDescription(SmallVectorImpl<char> &scratch) const {
+    return Info->getArgDescription(scratch);
+  }
+
   /// \returns The interface type for the function being applied.
   Type getFnInterfaceType() const { return Info->getFnInterfaceType(); }
 

test/IDE/infer_import_as_member.swift

@@ -126,10 +126,10 @@ func testNonEphemeralInitParams(x: Double) {
   var x = x
 
   _ = IAMPointerStruct(ptr1: &x, ptr2: &x)
-  // expected-error@-1 {{cannot use inout expression here; argument #1 must be a pointer that outlives the call to 'init(ptr1:ptr2:)'}}
+  // expected-error@-1 {{cannot use inout expression here; argument 'ptr1' must be a pointer that outlives the call to 'init(ptr1:ptr2:)'}}
   // expected-note@-2 {{implicit argument conversion from 'Double' to 'UnsafeMutablePointer<Double>?' produces a pointer valid only for the duration of the call to 'init(ptr1:ptr2:)'}}
   // expected-note@-3 {{use 'withUnsafeMutablePointer' in order to explicitly convert argument to pointer valid for a defined scope}}
-  // expected-error@-4 {{cannot use inout expression here; argument #2 must be a pointer that outlives the call to 'init(ptr1:ptr2:)'}}
+  // expected-error@-4 {{cannot use inout expression here; argument 'ptr2' must be a pointer that outlives the call to 'init(ptr1:ptr2:)'}}
   // expected-note@-5 {{implicit argument conversion from 'Double' to 'UnsafeMutablePointer<Double>?' produces a pointer valid only for the duration of the call to 'init(ptr1:ptr2:)'}}
   // expected-note@-6 {{use 'withUnsafeMutablePointer' in order to explicitly convert argument to pointer valid for a defined scope}}
 

test/IDE/newtype.swift

@@ -242,15 +242,15 @@ func testNonEphemeralInitParams(x: OpaquePointer) {
   // expected-note@-1 {{implicit argument conversion from 'OpaquePointer' to 'UnsafeMutablePointer<OpaquePointer>' produces a pointer valid only for the duration of the call}}
   // expected-note@-2 {{use 'withUnsafeMutablePointer' in order to explicitly convert argument to pointer valid for a defined scope}}
 
-  _ = TRefRef(rawValue: &x) // expected-warning {{inout expression creates a temporary pointer, but argument #1 should be a pointer that outlives the call to 'init(rawValue:)'}}
+  _ = TRefRef(rawValue: &x) // expected-warning {{inout expression creates a temporary pointer, but argument 'rawValue' should be a pointer that outlives the call to 'init(rawValue:)'}}
   // expected-note@-1 {{implicit argument conversion from 'OpaquePointer' to 'UnsafeMutablePointer<OpaquePointer>' produces a pointer valid only for the duration of the call}}
   // expected-note@-2 {{use 'withUnsafeMutablePointer' in order to explicitly convert argument to pointer valid for a defined scope}}
 
   _ = ConstTRefRef(&x) // expected-warning {{inout expression creates a temporary pointer, but argument #1 should be a pointer that outlives the call to 'init(_:)'}}
   // expected-note@-1 {{implicit argument conversion from 'OpaquePointer' to 'UnsafePointer<OpaquePointer>' produces a pointer valid only for the duration of the call}}
   // expected-note@-2 {{use 'withUnsafePointer' in order to explicitly convert argument to pointer valid for a defined scope}}
 
-  _ = ConstTRefRef(rawValue: &x) // expected-warning {{inout expression creates a temporary pointer, but argument #1 should be a pointer that outlives the call to 'init(rawValue:)'}}
+  _ = ConstTRefRef(rawValue: &x) // expected-warning {{inout expression creates a temporary pointer, but argument 'rawValue' should be a pointer that outlives the call to 'init(rawValue:)'}}
   // expected-note@-1 {{implicit argument conversion from 'OpaquePointer' to 'UnsafePointer<OpaquePointer>' produces a pointer valid only for the duration of the call}}
   // expected-note@-2 {{use 'withUnsafePointer' in order to explicitly convert argument to pointer valid for a defined scope}}
 }

test/Sema/diag_non_ephemeral.swift

@@ -374,29 +374,29 @@ func testNonEphemeralInMembers() {
   // expected-note@-1 {{implicit argument conversion from '[Int]' to 'UnsafePointer<Int>' produces a pointer valid only for the duration of the call to 'takesConstStaticAndReturns'}}
   // expected-note@-2 {{use the 'withUnsafeBufferPointer' method on Array in order to explicitly convert argument to buffer pointer valid for a defined scope}}
 
-  S2.takesMutableRawStatic(ptr: &local) // expected-error {{cannot use inout expression here; argument #1 must be a pointer that outlives the call to 'takesMutableRawStatic(_:ptr:)'}}
+  S2.takesMutableRawStatic(ptr: &local) // expected-error {{cannot use inout expression here; argument 'ptr' must be a pointer that outlives the call to 'takesMutableRawStatic(_:ptr:)'}}
   // expected-note@-1 {{implicit argument conversion from 'Int' to 'UnsafeMutableRawPointer' produces a pointer valid only for the duration of the call to 'takesMutableRawStatic(_:ptr:)'}}
   // expected-note@-2 {{use 'withUnsafeMutableBytes' in order to explicitly convert argument to buffer pointer valid for a defined scope}}
 
-  S2.takesMutableRawStatic("", ptr: &local) // expected-error {{cannot use inout expression here; argument #2 must be a pointer that outlives the call to 'takesMutableRawStatic(_:ptr:)'}}
+  S2.takesMutableRawStatic("", ptr: &local) // expected-error {{cannot use inout expression here; argument 'ptr' must be a pointer that outlives the call to 'takesMutableRawStatic(_:ptr:)'}}
   // expected-note@-1 {{implicit argument conversion from 'Int' to 'UnsafeMutableRawPointer' produces a pointer valid only for the duration of the call to 'takesMutableRawStatic(_:ptr:)'}}
   // expected-note@-2 {{use 'withUnsafeMutableBytes' in order to explicitly convert argument to buffer pointer valid for a defined scope}}
 
   var s2 = S2()
   s2.takesMutableRaw()
-  s2.takesMutableRaw(ptr: &local) // expected-error {{cannot use inout expression here; argument #1 must be a pointer that outlives the call to 'takesMutableRaw(ptr:)'}}
+  s2.takesMutableRaw(ptr: &local) // expected-error {{cannot use inout expression here; argument 'ptr' must be a pointer that outlives the call to 'takesMutableRaw(ptr:)'}}
   // expected-note@-1 {{implicit argument conversion from 'Int' to 'UnsafeMutableRawPointer' produces a pointer valid only for the duration of the call to 'takesMutableRaw(ptr:)'}}
   // expected-note@-2 {{use 'withUnsafeMutableBytes' in order to explicitly convert argument to buffer pointer valid for a defined scope}}
 
-  _ = s2[takesConstInt8: ""] // expected-error {{cannot pass 'String' to parameter; argument #1 must be a pointer that outlives the call to 'subscript(takesConstInt8:)'}}
+  _ = s2[takesConstInt8: ""] // expected-error {{cannot pass 'String' to parameter; argument 'takesConstInt8' must be a pointer that outlives the call to 'subscript(takesConstInt8:)'}}
   // expected-note@-1 {{implicit argument conversion from 'String' to 'UnsafePointer<Int8>' produces a pointer valid only for the duration of the call to 'subscript(takesConstInt8:)'}}
   // expected-note@-2 {{use the 'withCString' method on String in order to explicitly convert argument to pointer valid for a defined scope}}
 
-  s2[takesConstInt8: ""] += 1 // expected-error {{cannot pass 'String' to parameter; argument #1 must be a pointer that outlives the call to 'subscript(takesConstInt8:)'}}
+  s2[takesConstInt8: ""] += 1 // expected-error {{cannot pass 'String' to parameter; argument 'takesConstInt8' must be a pointer that outlives the call to 'subscript(takesConstInt8:)'}}
   // expected-note@-1 {{implicit argument conversion from 'String' to 'UnsafePointer<Int8>' produces a pointer valid only for the duration of the call to 'subscript(takesConstInt8:)'}}
   // expected-note@-2 {{use the 'withCString' method on String in order to explicitly convert argument to pointer valid for a defined scope}}
 
-  _ = \S2.[takesConstInt8: ""] // expected-error {{cannot pass 'String' to parameter; argument #1 must be a pointer that outlives the call to 'subscript(takesConstInt8:)'}}
+  _ = \S2.[takesConstInt8: ""] // expected-error {{cannot pass 'String' to parameter; argument 'takesConstInt8' must be a pointer that outlives the call to 'subscript(takesConstInt8:)'}}
   // expected-note@-1 {{implicit argument conversion from 'String' to 'UnsafePointer<Int8>' produces a pointer valid only for the duration of the call to 'subscript(takesConstInt8:)'}}
   // expected-note@-2 {{use the 'withCString' method on String in order to explicitly convert argument to pointer valid for a defined scope}}
 }
@@ -451,11 +451,11 @@ enum E {
 func testNonEphemeralInMemberwiseInits() {
   var local = 0
 
-  _ = S3(ptr1: &topLevelS, ptr2: &local) // expected-error {{cannot use inout expression here; argument #2 must be a pointer that outlives the call to 'init(ptr1:ptr2:)'}}
+  _ = S3(ptr1: &topLevelS, ptr2: &local) // expected-error {{cannot use inout expression here; argument 'ptr2' must be a pointer that outlives the call to 'init(ptr1:ptr2:)'}}
   // expected-note@-1 {{implicit argument conversion from 'Int' to 'UnsafeMutableRawPointer?' produces a pointer valid only for the duration of the call to 'init(ptr1:ptr2:)'}}
   // expected-note@-2 {{use 'withUnsafeMutableBytes' in order to explicitly convert argument to buffer pointer valid for a defined scope}}
 
-  _ = S3.init(ptr1: &local, ptr2: &topLevelS) // expected-error {{cannot use inout expression here; argument #1 must be a pointer that outlives the call to 'init(ptr1:ptr2:)'}}
+  _ = S3.init(ptr1: &local, ptr2: &topLevelS) // expected-error {{cannot use inout expression here; argument 'ptr1' must be a pointer that outlives the call to 'init(ptr1:ptr2:)'}}
   // expected-note@-1 {{implicit argument conversion from 'Int' to 'UnsafeMutableRawPointer' produces a pointer valid only for the duration of the call to 'init(ptr1:ptr2:)'}}
   // expected-note@-2 {{use 'withUnsafeMutableBytes' in order to explicitly convert argument to buffer pointer valid for a defined scope}}
 
@@ -505,3 +505,18 @@ func testNonEphemeralErrorDoesntAffectOverloadResolution() {
   // expected-note@-1 {{implicit argument conversion from '[Int]' to 'UnsafePointer<Int>' produces a pointer valid only for the duration of the call to 'takesPointerOverload2'}}
   // expected-note@-2 {{use the 'withUnsafeBufferPointer' method on Array in order to explicitly convert argument to buffer pointer valid for a defined scope}}
 }
+
+func takesTwoPointers(@_nonEphemeral ptr _: UnsafePointer<Int>, @_nonEphemeral ptr _: UnsafePointer<Int>) {}
+
+func testArgumentLabelReferencing() {
+  // Because takesTwoPointers has two argument labels with the same name, refer
+  // to the argument by position.
+  var arr = [1, 2, 3]
+  takesTwoPointers(ptr: arr, ptr: arr)
+  // expected-error@-1 {{cannot pass '[Int]' to parameter; argument #1 must be a pointer that outlives the call to 'takesTwoPointers(ptr:ptr:)'}}
+  // expected-note@-2 {{implicit argument conversion from '[Int]' to 'UnsafePointer<Int>' produces a pointer valid only for the duration of the call to 'takesTwoPointers(ptr:ptr:)'}}
+  // expected-note@-3 {{use the 'withUnsafeBufferPointer' method on Array in order to explicitly convert argument to buffer pointer valid for a defined scope}}
+  // expected-error@-4 {{cannot pass '[Int]' to parameter; argument #2 must be a pointer that outlives the call to 'takesTwoPointers(ptr:ptr:)'}}
+  // expected-note@-5 {{implicit argument conversion from '[Int]' to 'UnsafePointer<Int>' produces a pointer valid only for the duration of the call to 'takesTwoPointers(ptr:ptr:)'}}
+  // expected-note@-6 {{use the 'withUnsafeBufferPointer' method on Array in order to explicitly convert argument to buffer pointer valid for a defined scope}}
+}