Merge pull request #4114 from atrick/rawptr-diag

Improve diagnostics on invalid UnsafePointer conversion.

Author: tkremenek

include/swift/AST/DiagnosticsSema.def

@@ -210,8 +210,9 @@ ERROR(cannot_call_with_params, none,
       "cannot invoke %select{|initializer for type }2'%0' with an argument list"
       " of type '%1'", (StringRef, StringRef, bool))
 
-NOTE(pointer_init_add_mutating,none,
-     "do you want to add 'mutating'", ())
+NOTE(pointer_init_to_type,none,
+      "Pointer conversion restricted: use '.assumingMemoryBound(to:)' or '.bindMemory(to:capacity:)' to view memory as a type.", ())
+
 ERROR(expected_do_in_statement,none,
       "expected 'do' keyword to designate a block of statements", ())
 

lib/Sema/CSDiag.cpp

@@ -4976,26 +4976,37 @@ bool FailureDiagnosis::diagnoseNilLiteralComparison(
   return true;
 }
 
-/// When initializing UnsafeMutablePointer from a given UnsafePointer, we need
-/// to insert "mutating:" label before the argument to ensure the correct
-/// intializer gets called. This function checks if we need to add the label.
-static bool shouldAddMutating(ASTContext &Ctx, const Expr *Fn,
-                              const Expr* Arg) {
+/// When initializing Unsafe[Mutable]Pointer<T> from Unsafe[Mutable]RawPointer,
+/// issue a diagnostic that refers to the API for binding memory to a type.
+static bool isCastToTypedPointer(ASTContext &Ctx, const Expr *Fn,
+                                 const Expr* Arg) {
   auto *TypeExp = dyn_cast<TypeExpr>(Fn);
   auto *ParenExp = dyn_cast<ParenExpr>(Arg);
   if (!TypeExp || !ParenExp)
     return false;
+
   auto InitType = TypeExp->getInstanceType();
   auto ArgType = ParenExp->getSubExpr()->getType();
   if (InitType.isNull() || ArgType.isNull())
     return false;
-  if (auto *InitNom = InitType->getAnyNominal()) {
-    if (auto *ArgNom = ArgType->getAnyNominal()) {
-      return InitNom == Ctx.getUnsafeMutablePointerDecl() &&
-        ArgNom == Ctx.getUnsafePointerDecl();
-    }
+
+  auto *InitNom = InitType->getAnyNominal();
+  if (!InitNom)
+    return false;
+
+  if (InitNom != Ctx.getUnsafeMutablePointerDecl()
+      && InitNom != Ctx.getUnsafePointerDecl()) {
+    return false;
   }
-  return false;
+  auto *ArgNom = ArgType->getAnyNominal();
+  if (!ArgNom)
+    return false;
+
+  if (ArgNom != Ctx.getUnsafeMutableRawPointerDecl()
+      && ArgNom != Ctx.getUnsafeRawPointerDecl()) {
+    return false;
+  }
+  return true;
 }
 
 bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
@@ -5313,10 +5324,9 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
              overloadName, argString, isInitializer);
   }
 
-
-  if (shouldAddMutating(CS->DC->getASTContext(), fnExpr, argExpr)) {
-    diagnose(fnExpr->getLoc(), diag::pointer_init_add_mutating).fixItInsert(
-      cast<ParenExpr>(argExpr)->getSubExpr()->getStartLoc(), "mutating: ");
+  if (isCastToTypedPointer(CS->DC->getASTContext(), fnExpr, argExpr)) {
+    diagnose(fnExpr->getLoc(), diag::pointer_init_to_type)
+      .highlight(argExpr->getSourceRange());
   }
 
   // Did the user intend on invoking a different overload?

stdlib/public/core/UnsafePointer.swift.gyb

@@ -532,7 +532,7 @@ extension ${Self} {
     "use 'withMemoryRebound(to:capacity:_)' to temporarily view memory as another layout-compatible type.")
   public init?<U>(_ from : UnsafePointer<U>?) { Builtin.unreachable(); return nil }
 
-% if mutable:
+ % if mutable:
   @available(*, unavailable, renamed: "init(mutating:)")
   public init(_ from : UnsafePointer<Pointee>) { Builtin.unreachable() }
 

test/1_stdlib/UnsafePointerDiagnostics.swift

@@ -2,15 +2,11 @@
 
 // Test availability attributes on UnsafePointer initializers.
 // Assume the original source contains no UnsafeRawPointer types.
-//
-// TODO:
-// - implement the Unsafe[Mutable]Pointer<Void> to Unsafe[Mutable]RawPointer rename
-// - test multiple fix-its per line: type rename + initializer rename/diagnostic
 func unsafePointerConversionAvailability(
   mrp: UnsafeMutableRawPointer,
   rp: UnsafeRawPointer,
-  umpv: UnsafeMutablePointer<Void>,
-  upv: UnsafePointer<Void>,
+  umpv: UnsafeMutablePointer<Void>, // FIXME: This should yield a warning to rename UnsafeMutablePointer<Void> to UnsafeMutableRawPointer
+  upv: UnsafePointer<Void>,         // FIXME: This should yield a warning to rename UnsafePointer<Void> to UnsafeRawPointer
   umpi: UnsafeMutablePointer<Int>,
   upi: UnsafePointer<Int>,
   umps: UnsafeMutablePointer<String>,
@@ -35,42 +31,39 @@ func unsafePointerConversionAvailability(
   _ = UnsafeRawPointer(umps)
   _ = UnsafeRawPointer(ups)
 
-  // FIXME: All of these should yield a fix-it to rename
-  // UnsafeMutablePointer<Void> to UnsafeMutableRawPointer(umpv)
-  _ = UnsafeMutablePointer<Void>(rp) // expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Void>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{}}
-  _ = UnsafeMutablePointer<Void>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Void>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{}}
+  // FIXME: All of these should yield a warning to rename
+  // UnsafeMutablePointer<Void> to UnsafeMutableRawPointer.
+  _ = UnsafeMutablePointer<Void>(rp) // expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Void>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{Pointer conversion restricted: use '.assumingMemoryBound(to:)' or '.bindMemory(to:capacity:)' to view memory as a type.}} expected-note {{}}
+  _ = UnsafeMutablePointer<Void>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Void>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{Pointer conversion restricted: use '.assumingMemoryBound(to:)' or '.bindMemory(to:capacity:)' to view memory as a type.}} expected-note {{}}
   _ = UnsafeMutablePointer<Void>(umpv)
   _ = UnsafeMutablePointer<Void>(upv)  // expected-error {{'init' has been renamed to 'init(mutating:)'}}
   _ = UnsafeMutablePointer<Void>(umpi)
   _ = UnsafeMutablePointer<Void>(upi)  // expected-error {{'init' has been renamed to 'init(mutating:)'}}
   _ = UnsafeMutablePointer<Void>(umps)
   _ = UnsafeMutablePointer<Void>(ups)  // expected-error {{'init' has been renamed to 'init(mutating:)'}}
 
-  // FIXME: All of these should yield a fix-it to rename
-  // UnsafePointer<Void> to UnsafeRawPointer(umpv)
-  _ = UnsafePointer<Void>(rp)  // expected-error {{cannot invoke initializer for type 'UnsafePointer<Void>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{}}
-  _ = UnsafePointer<Void>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafePointer<Void>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{}}
-  _ = UnsafePointer<Void>(umpv) 
+  // FIXME: All of these should yield a warning to rename
+  // UnsafePointer<Void> to UnsafeRawPointer.
+  _ = UnsafePointer<Void>(rp)  // expected-error {{cannot invoke initializer for type 'UnsafePointer<Void>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{Pointer conversion restricted: use '.assumingMemoryBound(to:)' or '.bindMemory(to:capacity:)' to view memory as a type.}} expected-note {{}}
+  _ = UnsafePointer<Void>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafePointer<Void>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{Pointer conversion restricted: use '.assumingMemoryBound(to:)' or '.bindMemory(to:capacity:)' to view memory as a type.}} expected-note {{}}
+  _ = UnsafePointer<Void>(umpv)
   _ = UnsafePointer<Void>(upv)
   _ = UnsafePointer<Void>(umpi)
   _ = UnsafePointer<Void>(upi)
   _ = UnsafePointer<Void>(umps)
   _ = UnsafePointer<Void>(ups)
 
-  // FIXME: Conversion from UnsafePointer<Void> or UnsafeRawPointer to a typed
-  // pointer should have a diagnostic: 'init' is unavailable: Conversion
-  // restricted. Use 'assumingMemoryBound(to:)' or 'bindMemory(to:capacity:)'
-  _ = UnsafeMutablePointer<Int>(rp) // expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Int>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{}}
-  _ = UnsafeMutablePointer<Int>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Int>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{}}
+  _ = UnsafeMutablePointer<Int>(rp) // expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Int>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{Pointer conversion restricted: use '.assumingMemoryBound(to:)' or '.bindMemory(to:capacity:)' to view memory as a type.}} expected-note {{}}
+  _ = UnsafeMutablePointer<Int>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Int>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{Pointer conversion restricted: use '.assumingMemoryBound(to:)' or '.bindMemory(to:capacity:)' to view memory as a type.}} expected-note {{}}
   _ = UnsafeMutablePointer<Int>(umpv) // expected-error {{'init' is unavailable: use 'withMemoryRebound(to:capacity:_)' to temporarily view memory as another layout-compatible type.}}
   _ = UnsafeMutablePointer<Int>(upv)  // expected-error {{'init' is unavailable: use 'withMemoryRebound(to:capacity:_)' to temporarily view memory as another layout-compatible type.}}
   _ = UnsafeMutablePointer<Int>(umpi)
   _ = UnsafeMutablePointer<Int>(upi)  // expected-error {{'init' has been renamed to 'init(mutating:)'}}
   _ = UnsafeMutablePointer<Int>(umps) // expected-error {{'init' is unavailable: use 'withMemoryRebound(to:capacity:_)' to temporarily view memory as another layout-compatible type.}}
   _ = UnsafeMutablePointer<Int>(ups)  // expected-error {{'init' is unavailable: use 'withMemoryRebound(to:capacity:_)' to temporarily view memory as another layout-compatible type.}}
 
-  _ = UnsafePointer<Int>(rp)  // expected-error {{cannot invoke initializer for type 'UnsafePointer<Int>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{}}
-  _ = UnsafePointer<Int>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafePointer<Int>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{}}
+  _ = UnsafePointer<Int>(rp)  // expected-error {{cannot invoke initializer for type 'UnsafePointer<Int>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{Pointer conversion restricted: use '.assumingMemoryBound(to:)' or '.bindMemory(to:capacity:)' to view memory as a type.}} expected-note {{}}
+  _ = UnsafePointer<Int>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafePointer<Int>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{Pointer conversion restricted: use '.assumingMemoryBound(to:)' or '.bindMemory(to:capacity:)' to view memory as a type.}} expected-note {{}}
   _ = UnsafePointer<Int>(umpv) // expected-error {{'init' is unavailable: use 'withMemoryRebound(to:capacity:_)' to temporarily view memory as another layout-compatible type.}}
   _ = UnsafePointer<Int>(upv)  // expected-error {{'init' is unavailable: use 'withMemoryRebound(to:capacity:_)' to temporarily view memory as another layout-compatible type.}}
   _ = UnsafePointer<Int>(umpi)