Merge pull request swiftlang#63406 from xedin/rdar-104931999-5.8

[5.8][SILOptimizer] Don't diagnose unreachable instructions that belong to…

Author: xedin

SwiftCompilerSources/Sources/SIL/Location.swift

@@ -12,11 +12,24 @@
 
 import SILBridging
 
-public struct Location {
+public struct Location: Equatable, CustomStringConvertible {
   let bridged: swift.SILDebugLocation
-  
+
+  public var description: String {
+    let stdString = SILLocation_debugDescription(bridged)
+    return String(_cxxString: stdString)
+  }
+
   /// Keeps the debug scope but marks it as auto-generated.
   public var autoGenerated: Location {
     Location(bridged: SILLocation_getAutogeneratedLocation(bridged))
   }
+
+  public static func ==(lhs: Location, rhs: Location) -> Bool {
+    SILLocation_equal(lhs.bridged, rhs.bridged)
+  }
+
+  public func hasSameSourceLocation(as other: Location) -> Bool {
+    SILLocation_hasSameSourceLocation(bridged, other.bridged)
+  }
 }

include/swift/SIL/SILBridging.h

@@ -331,8 +331,11 @@ llvm::StringRef SILType_getNominalFieldName(BridgedType type, SwiftInt index);
 SwiftInt SILType_getCaseIdxOfEnumType(BridgedType type,
                                       llvm::StringRef caseName);
 
+std::string SILLocation_debugDescription(swift::SILDebugLocation loc);
 swift::SILDebugLocation
 SILLocation_getAutogeneratedLocation(swift::SILDebugLocation loc);
+bool SILLocation_equal(swift::SILDebugLocation lhs, swift::SILDebugLocation rhs);
+bool SILLocation_hasSameSourceLocation(swift::SILDebugLocation lhs, swift::SILDebugLocation rhs);
 
 BridgedBasicBlock SILArgument_getParent(BridgedArgument argument);
 BridgedArgumentConvention SILArgument_getConvention(BridgedArgument argument);

include/swift/SIL/SILLocation.h

@@ -427,6 +427,7 @@ class SILLocation {
   /// Pretty-print the value.
   void dump() const;
   void print(raw_ostream &OS, const SourceManager &SM) const;
+  void print(raw_ostream &OS) const;
 
   inline bool operator==(const SILLocation& R) const {
     return kindAndFlags.packedKindAndFlags == R.kindAndFlags.packedKindAndFlags
@@ -435,6 +436,15 @@ class SILLocation {
 
   inline bool operator!=(const SILLocation &R) const { return !(*this == R); }
 
+  bool hasSameSourceLocation(const SILLocation &rhs) {
+    if (*this == rhs)
+      return true;
+    if (isASTNode() && rhs.isASTNode()) {
+      return getSourceLoc(getPrimaryASTNode()) == rhs.getSourceLoc(rhs.getPrimaryASTNode());
+    }
+    return false;
+  }
+
   friend llvm::hash_code hash_value(const SILLocation &);
 };
 

lib/SIL/IR/SILLocation.cpp

@@ -151,17 +151,17 @@ SILLocation::FilenameAndLocation *SILLocation::getCompilerGeneratedLoc() {
   return &compilerGenerated;
 }
 
-static void dumpSourceLoc(SourceLoc loc) {
+static void printSourceLoc(SourceLoc loc, raw_ostream &OS) {
   if (!loc.isValid()) {
-    llvm::dbgs() << "<invalid loc>";
+    OS << "<invalid loc>";
     return;
   }
   const char *srcPtr = (const char *)loc.getOpaquePointerValue();
   unsigned len = strnlen(srcPtr, 20);
   if (len < 20) {
-    llvm::dbgs() << '"' << StringRef(srcPtr, len) << '"';
+    OS << '"' << StringRef(srcPtr, len) << '"';
   } else {
-    llvm::dbgs() << '"' << StringRef(srcPtr, 20) << "[...]\"";
+    OS << '"' << StringRef(srcPtr, 20) << "[...]\"";
   }
 }
 
@@ -182,7 +182,7 @@ void SILLocation::dump() const {
   if (isFilenameAndLocation()) {
     getFilenameAndLocation()->dump();
   } else {
-    dumpSourceLoc(getSourceLoc());
+    printSourceLoc(getSourceLoc(), llvm::dbgs());
   }
 
   if (isAutoGenerated())     llvm::dbgs() << ":auto";
@@ -191,7 +191,7 @@ void SILLocation::dump() const {
   if (isSILFile())           llvm::dbgs() << ":sil";
   if (hasASTNodeForDebugging()) {
     llvm::dbgs() << ":debug[";
-    dumpSourceLoc(getSourceLocForDebugging());
+    printSourceLoc(getSourceLocForDebugging(), llvm::dbgs());
     llvm::dbgs() << "]\n";
   }
 }
@@ -206,6 +206,18 @@ void SILLocation::print(raw_ostream &OS, const SourceManager &SM) const {
   }
 }
 
+void SILLocation::print(raw_ostream &OS) const {
+  if (isNull()) {
+    OS << "<no loc>";
+  } else if (isFilenameAndLocation()) {
+    getFilenameAndLocation()->print(OS);
+  } else if (DeclContext *dc = getAsDeclContext()){
+    getSourceLoc().print(OS, dc->getASTContext().SourceMgr);
+  } else {
+    printSourceLoc(getSourceLoc(), OS);
+  }
+}
+
 RegularLocation::RegularLocation(Stmt *S, Pattern *P, SILModule &Module) :
   SILLocation(new (Module) ExtendedASTNodeLoc(S, P), RegularKind) {}
 

lib/SIL/Utils/SILBridging.cpp

@@ -607,12 +607,39 @@ SwiftInt SILType_getCaseIdxOfEnumType(BridgedType type,
 //                                SILLocation
 //===----------------------------------------------------------------------===//
 
+std::string SILLocation_debugDescription(swift::SILDebugLocation dloc) {
+  std::string str;
+  llvm::raw_string_ostream os(str);
+  SILLocation loc = dloc.getLocation();
+  loc.print(os);
+#ifndef NDEBUG
+  if (const SILDebugScope *scope = dloc.getScope()) {
+    if (DeclContext *dc = loc.getAsDeclContext()) {
+      os << ", scope=";
+      scope->print(dc->getASTContext().SourceMgr, os, /*indent*/ 2);
+    } else {
+      os << ", scope=?";
+    }
+  }
+#endif
+  return str;
+}
+
 SILDebugLocation SILLocation_getAutogeneratedLocation(SILDebugLocation loc) {
   SILDebugLocation autoGenLoc(RegularLocation::getAutoGeneratedLocation(),
                               loc.getScope());
   return autoGenLoc;
 }
 
+bool SILLocation_equal(swift::SILDebugLocation lhs, swift::SILDebugLocation rhs) {
+  return lhs.getLocation() == rhs.getLocation() && lhs.getScope() == rhs.getScope();
+}
+
+bool SILLocation_hasSameSourceLocation(swift::SILDebugLocation lhs, swift::SILDebugLocation rhs) {
+  return lhs.getLocation().hasSameSourceLocation(rhs.getLocation()) &&
+         lhs.getScope() == rhs.getScope();
+}
+
 //===----------------------------------------------------------------------===//
 //                            SILGlobalVariable
 //===----------------------------------------------------------------------===//

lib/SILOptimizer/Mandatory/DiagnoseUnreachable.cpp

@@ -745,6 +745,44 @@ static bool simplifyBlocksWithCallsToNoReturn(SILBasicBlock &BB,
   // function, the entire block is dead.
   NoReturnCall = getPrecedingCallToNoReturn(BB);
 
+  // Diagnose the unreachable code within the same block as the call to
+  // noreturn.
+  auto diagnoseUnreachableCode = [&](SILInstruction *noReturnCall,
+                                     SILInstruction *currInst) {
+    if (DiagnosedUnreachableCode)
+      return false;
+
+    // If current instruction belongs to the no-return call itself, skip it.
+    //
+    // It could happen when i.e. result has to be copied to be passed to
+    // some call.
+    if (currInst->getLoc().hasSameSourceLocation(noReturnCall->getLoc()))
+      return false;
+
+    if (!isUserCode(currInst))
+      return false;
+
+    // If we have an instruction that is an end_borrow, ignore it. This
+    // happens when passing a guaranteed argument through generic code paths
+    // to no return functions.
+    if (isa<EndBorrowInst>(currInst))
+      return false;
+
+    // If no-return instruction is not something we can point in code or
+    // it's an explicit cast, skip it.
+    if (!noReturnCall->getLoc().is<RegularLocation>() ||
+        noReturnCall->getLoc().isASTNode<ExplicitCastExpr>())
+      return false;
+
+    diagnose(BB.getModule().getASTContext(), currInst->getLoc().getSourceLoc(),
+             diag::unreachable_code);
+    diagnose(BB.getModule().getASTContext(),
+             noReturnCall->getLoc().getSourceLoc(),
+             diag::call_to_noreturn_note);
+
+    return true;
+  };
+
   // Does this block contain a call to a noreturn function?
   while (I != E) {
     auto *CurrentInst = &*I;
@@ -758,26 +796,8 @@ static bool simplifyBlocksWithCallsToNoReturn(SILBasicBlock &BB,
       // We will need to delete the instruction later on.
       ToBeDeleted.push_back(CurrentInst);
 
-      // Diagnose the unreachable code within the same block as the call to
-      // noreturn.
-      if (isUserCode(CurrentInst) && !DiagnosedUnreachableCode) {
-        // If we have an instruction that is an end_borrow, ignore it. This
-        // happens when passing a guaranteed argument through generic code paths
-        // to no return functions.
-        if (!isa<EndBorrowInst>(CurrentInst)) {
-          if (NoReturnCall->getLoc().is<RegularLocation>()) {
-            if (!NoReturnCall->getLoc().isASTNode<ExplicitCastExpr>()) {
-              diagnose(BB.getModule().getASTContext(),
-                       CurrentInst->getLoc().getSourceLoc(),
-                       diag::unreachable_code);
-              diagnose(BB.getModule().getASTContext(),
-                       NoReturnCall->getLoc().getSourceLoc(),
-                       diag::call_to_noreturn_note);
-              DiagnosedUnreachableCode = true;
-            }
-          }
-        }
-      }
+      DiagnosedUnreachableCode |=
+          diagnoseUnreachableCode(NoReturnCall, CurrentInst);
 
       // We are going to bluntly remove these instructions. Change uses in
       // different basic blocks to undef. This is safe because all control flow

test/SILOptimizer/diagnose_unreachable.swift

@@ -518,3 +518,55 @@ func keypathWithDynamicLookup() {
   // when used in conjunction with a dynamicMemberLookup enum.
   let _ = \DynamicLookupEnum.innerEnum // no warning
 }
+
+func test_no_warnings_with_fatalError_when_wrapped_in_buildExpression() {
+  enum Either<T,U> {
+    case first(T)
+    case second(U)
+  }
+
+  @resultBuilder
+  struct MyBuilder {
+    static func buildExpression<T>(_ e: T) -> T { e }
+    static func buildBlock() -> () { }
+
+    static func buildBlock<T1>(_ t1: T1) -> T1 {
+      return t1
+    }
+
+    static func buildBlock<T1, T2>(_ t1: T1, _ t2: T2) -> (T1, T2) {
+      return (t1, t2)
+    }
+
+    static func buildBlock<T1, T2, T3>(_ t1: T1, _ t2: T2, _ t3: T3) -> (T1, T2, T3) {
+      return (t1, t2, t3)
+    }
+
+    static func buildEither<T,U>(first value: T) -> Either<T, U> {
+      return .first(value)
+    }
+
+    static func buildEither<T,U>(second value: U) -> Either<T, U> {
+      return .second(value)
+    }
+  }
+
+  func test<T>(@MyBuilder _: (Int) -> T) {}
+
+  test {
+    if $0 < 0 {
+      fatalError() // ok, no warning even though fatalError() is wrapped
+    } else if $0 > 0 {
+      42
+    } else {
+      0
+    }
+  }
+
+  test {
+    switch $0 {
+    case 0: "0"
+    default: fatalError() // Ok, no warning even though fatalError() is wrapped
+    }
+  }
+}