-Wunreachable-code fixes

[thakis]

Summary

not for review

clang/lib/Sema/SemaExpr.cpp

   case IncompatibleBlockPointer:
     DiagKind = diag::err_typecheck_convert_incompatible_block_pointer;
     isInvalid = true;
     break;
   case IncompatibleObjCQualifiedId: {
     if (SrcType->isObjCQualifiedIdType()) {
       const ObjCObjectPointerType *srcOPT =
                 SrcType->castAs<ObjCObjectPointerType>();
-      for (auto *srcProto : srcOPT->quals()) {
+      for (auto *srcProto : srcOPT->quals()) {  // XXX
         PDecl = srcProto;
         break;
       }
       if (const ObjCInterfaceType *IFaceT =
             DstType->castAs<ObjCObjectPointerType>()->getInterfaceType())
         IFace = IFaceT->getDecl();
     }
     else if (DstType->isObjCQualifiedIdType()) {
       const ObjCObjectPointerType *dstOPT =
         DstType->castAs<ObjCObjectPointerType>();
       for (auto *dstProto : dstOPT->quals()) {
-        PDecl = dstProto;
+        PDecl = dstProto;  // XXX
         break;
       }
       if (const ObjCInterfaceType *IFaceT =
             SrcType->castAs<ObjCObjectPointerType>()->getInterfaceType())
         IFace = IFaceT->getDecl();
     }
     if (getLangOpts().CPlusPlus) {
       DiagKind = diag::err_incompatible_qualified_id;

clang/lib/Serialization/ASTReader.cpp

           return OutOfDate;
         }
       }
     }
 
     // Check any additional module map files that are in the pcm, but not
     // found in header search. Cases that match are already removed.
     for (const FileEntry *ModMap : AdditionalStoredMaps) {
-      if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
+      if ((ClientLoadCapabilities & ARR_OutOfDate) == 0) {
         Diag(diag::err_module_different_modmap)
           << F.ModuleName << /*not new*/1 << ModMap->getName();
         return OutOfDate;
+      }
     }
   }
 
   if (Listener)
     Listener->ReadModuleMapFile(F.ModuleMapPath);
   return Success;
 }
 

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

static bool potentiallyWritesIntoIvar(const Decl *Parent,
   if (!Parent || !Parent->hasBody())
     return false;
   StatementMatcher WriteIntoIvarM = binaryOperator(
       hasOperatorName("="),
       hasLHS(ignoringParenImpCasts(
           objcIvarRefExpr(hasDeclaration(equalsNode(Ivar))).bind(IvarBind))));
   StatementMatcher ParentM = stmt(hasDescendant(WriteIntoIvarM));
   auto Matches = match(ParentM, *Parent->getBody(), Parent->getASTContext());
-  for (BoundNodes &Match : Matches) {
-    auto IvarRef = Match.getNodeAs<ObjCIvarRefExpr>(IvarBind);
+  if (!Matches.empty()) {
+    auto IvarRef = Matches.first().getNodeAs<ObjCIvarRefExpr>(IvarBind);
     if (IvarRef->isFreeIvar())
       return true;
 
     const Expr *Base = IvarRef->getBase();
     if (const auto *ICE = dyn_cast<ImplicitCastExpr>(Base))
       Base = ICE->getSubExpr();
 
     if (const auto *DRE = dyn_cast<DeclRefExpr>(Base))

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

       Src.push_back(N);
     }
     return DumpGraph(Src, Filename);
   } else {
     return llvm::WriteGraph(&G, "ExprEngine", /*ShortNames=*/false,
                             /*Title=*/"Exploded Graph",
                             /*Filename=*/std::string(Filename));
   }
-#endif
+#else
   llvm::errs() << "Warning: dumping graph requires assertions" << "\n";
   return "";
+#endif
 }
 
 std::string ExprEngine::DumpGraph(ArrayRef<const ExplodedNode*> Nodes,
                                   StringRef Filename) {
 #ifndef NDEBUG
   std::unique_ptr<ExplodedGraph> TrimmedG(G.trim(Nodes));
 
   if (!TrimmedG.get()) {

clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp

void addUSRsOfInstantiatedMethods(const CXXMethodDecl *MethodDecl) {
     for (const auto *Method : InstantiatedMethods) {
       if (USRSet.find(getUSRForDecl(
               Method->getInstantiatedFromMemberFunction())) != USRSet.end())
         USRSet.insert(getUSRForDecl(Method));
     }
   }
 
   bool checkIfOverriddenFunctionAscends(const CXXMethodDecl *MethodDecl) {
-    for (const auto &OverriddenMethod : MethodDecl->overridden_methods()) {
-      if (USRSet.find(getUSRForDecl(OverriddenMethod)) != USRSet.end())
-        return true;
-      return checkIfOverriddenFunctionAscends(OverriddenMethod);
-    }
+    ASTContext::overridden_methods_range OverriddenMethods =
+      MethodDecl->overridden_methods();
+    if (range.empty())
       return false;
+    if (USRSet.find(getUSRForDecl(*OverriddenMethods->begin())) != USRSet.end())
+      return true;
+    return checkIfOverriddenFunctionAscends(*OverriddenMethods->begin());
   }
 
   const Decl *FoundDecl;
   ASTContext &Context;
   std::set<std::string> USRSet;
   std::vector<const CXXMethodDecl *> OverriddenMethods;
   std::vector<const CXXMethodDecl *> InstantiatedMethods;
   std::vector<const ClassTemplatePartialSpecializationDecl *> PartialSpecs;

llvm/cmake/modules/HandleLLVMOptions.cmake

CHECK_CXX_SOURCE_COMPILES("class base {public: virtual void anchor();protected: ~base();};
                                class derived final : public base { public: ~derived();};
                                int main() { return 0; }"
                               CXX_WONT_WARN_ON_FINAL_NONVIRTUALDTOR)
     set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
     append_if(CXX_WONT_WARN_ON_FINAL_NONVIRTUALDTOR
               "-Wnon-virtual-dtor" CMAKE_CXX_FLAGS)
   endif()
 
+  if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+    append("-Wunreachable-code" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+  endif()
+
   # Enable -Wdelete-non-virtual-dtor if available.
   add_flag_if_supported("-Wdelete-non-virtual-dtor" DELETE_NON_VIRTUAL_DTOR_FLAG)
 
   # Check if -Wcomment is OK with an // comment ending with '\' if the next
   # line is also a // comment.
   set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
   set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -Werror -Wcomment")
   CHECK_C_SOURCE_COMPILES("// \\\\\\n//\\nint main() {return 0;}"

llvm/include/llvm/TextAPI/MachO/InterfaceFile.h

public:
   /// Set the parent umbrella frameworks.
   /// \param Target_ The target applicable to Parent
   /// \param Parent  The name of Parent
   void addParentUmbrella(const Target &Target_, StringRef Parent);
   const std::vector<std::pair<Target, std::string>> &umbrellas() const {
     return ParentUmbrellas;
   }
 
-  /// Get the parent umbrella framework.
-  const std::vector<std::pair<Target, std::string>> getParentUmbrellas() const {
-    return ParentUmbrellas;
-  }
-
   /// Add an allowable client.
   ///
   /// Mach-O Dynamic libraries have the concept of allowable clients that are
   /// checked during static link time. The name of the application or library
   /// that is being generated needs to match one of the allowable clients or the
   /// linker refuses to link this library.
   ///
   /// \param InstallName The name of the client that is allowed to link this library.

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

 }
 
 bool CombinerHelper::isPredecessor(MachineInstr &DefMI, MachineInstr &UseMI) {
   assert(DefMI.getParent() == UseMI.getParent());
   if (&DefMI == &UseMI)
     return false;
 
   // Loop through the basic block until we find one of the instructions.
+  // XXX what
   MachineBasicBlock::const_iterator I = DefMI.getParent()->begin();
   for (; &*I != &DefMI && &*I != &UseMI; ++I)
     return &*I == &DefMI;
 
   llvm_unreachable("Block must contain instructions");
 }
 
 bool CombinerHelper::dominates(MachineInstr &DefMI, MachineInstr &UseMI) {

llvm/lib/CodeGen/MachineVerifier.cpp

                  << "\n";
         }
     }
     break;
   }
   case TargetOpcode::STATEPOINT:
     if (!MI->getOperand(StatepointOpers::IDPos).isImm() ||
         !MI->getOperand(StatepointOpers::NBytesPos).isImm() ||
-        !MI->getOperand(StatepointOpers::NCallArgsPos).isImm())
+        !MI->getOperand(StatepointOpers::NCallArgsPos).isImm()) {
       report("meta operands to STATEPOINT not constant!", MI);
       break;
+    }
 
     auto VerifyStackMapConstant = [&](unsigned Offset) {
       if (!MI->getOperand(Offset).isImm() ||
           MI->getOperand(Offset).getImm() != StackMaps::ConstantOp ||
           !MI->getOperand(Offset + 1).isImm())
         report("stack map constant to STATEPOINT not well formed!", MI);
     };
     const unsigned VarStart = StatepointOpers(MI).getVarIdx();

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

                              TargetLookupError.c_str());
   RegInfo.reset(TheTarget->createMCRegInfo(TT.str()));
   return Error::success();
 }
 
 uint8_t DWARFContext::getCUAddrSize() {
   // In theory, different compile units may have different address byte
   // sizes, but for simplicity we just use the address byte size of the
-  // last compile unit. In practice the address size field is repeated across
+  // first compile unit. In practice the address size field is repeated across
   // various DWARF headers (at least in version 5) to make it easier to dump
   // them independently, not to enable varying the address size.
-  uint8_t Addr = 0;
-  for (const auto &CU : compile_units()) {
-    Addr = CU->getAddressByteSize();
-    break;
-  }
-  return Addr;
+  unit_iterator_range CUs = compile_units();
+  return CUs.empty() ? 0 : Addr = CUs->begin()->getAddressByteSize();
 }
 

llvm/lib/Object/XCOFFObjectFile.cpp

 
 void XCOFFObjectFile::moveRelocationNext(DataRefImpl &Rel) const {
   llvm_unreachable("Not yet implemented!");
   return;
 }
 
 uint64_t XCOFFObjectFile::getRelocationOffset(DataRefImpl Rel) const {
   llvm_unreachable("Not yet implemented!");
-  uint64_t Result = 0;
+  uint64_t Result;
   return Result;
 }
 
 symbol_iterator XCOFFObjectFile::getRelocationSymbol(DataRefImpl Rel) const {
   llvm_unreachable("Not yet implemented!");
   return symbol_iterator(SymbolRef());
 }
 
 uint64_t XCOFFObjectFile::getRelocationType(DataRefImpl Rel) const {
   llvm_unreachable("Not yet implemented!");
-  uint64_t Result = 0;
+  uint64_t Result;
   return Result;
 }
 
 void XCOFFObjectFile::getRelocationTypeName(
     DataRefImpl Rel, SmallVectorImpl<char> &Result) const {
   llvm_unreachable("Not yet implemented!");
   return;
 }

llvm/lib/TextAPI/MachO/TextStub.cpp

NormalizedTBD(IO &IO, const InterfaceFile *&File) {
         Flags |= TBDFlags::NotApplicationExtensionSafe;
 
       if (!File->isTwoLevelNamespace())
         Flags |= TBDFlags::FlatNamespace;
 
       if (File->isInstallAPI())
         Flags |= TBDFlags::InstallAPI;
 
-      for (const auto &Iter : File->umbrellas()) {
-        ParentUmbrella = Iter.second;
-        break;
-      }
+      if (!File->umbrellas().empty())
+        ParentUmbrella = File->umbrellas()->begin().second;
 
       std::set<ArchitectureSet> ArchSet;
       for (const auto &Library : File->allowableClients())
         ArchSet.insert(Library.getArchitectures());
 
       for (const auto &Library : File->reexportedLibraries())
         ArchSet.insert(Library.getArchitectures());
 

llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp

 // Return true if the call-site has an argument which is a PHI with only
 // constant incoming values.
 static bool isPredicatedOnPHI(CallSite CS) {
   Instruction *Instr = CS.getInstruction();
   BasicBlock *Parent = Instr->getParent();
   if (Instr != Parent->getFirstNonPHIOrDbg())
     return false;
 
-  for (auto &BI : *Parent) {
-    if (PHINode *PN = dyn_cast<PHINode>(&BI)) {
+  if (!Parent->empty) {
+    if (PHINode *PN = dyn_cast<PHINode>(&Parent->first())) {
       for (auto &I : CS.args())
         if (&*I == PN) {
           assert(PN->getNumIncomingValues() == 2 &&
                  "Unexpected number of incoming values");
           if (PN->getIncomingBlock(0) == PN->getIncomingBlock(1))
             return false;
           if (PN->getIncomingValue(0) == PN->getIncomingValue(1))
             continue;
           if (isa<Constant>(PN->getIncomingValue(0)) &&
               isa<Constant>(PN->getIncomingValue(1)))
             return true;
         }
     }
-    break;
   }
   return false;
 }
 
 using PredsWithCondsTy = SmallVector<std::pair<BasicBlock *, ConditionsTy>, 2>;
 
 // Check if any of the arguments in CS are predicated on a PHI node and return
 // the set of predecessors we should use for splitting.

llvm/tools/llvm-cvtres/llvm-cvtres.cpp

 };
 }
 
 LLVM_ATTRIBUTE_NORETURN void reportError(Twine Msg) {
   errs() << Msg;
   exit(1);
 }
 
-static void reportError(StringRef Input, std::error_code EC) {
+LLVM_ATTRIBUTE_NORETURN static void reportError(
+    StringRef Input, std::error_code EC) {
   reportError(Twine(Input) + ": " + EC.message() + ".\n");
 }
 
 void error(std::error_code EC) {
   if (!EC)
     return;
   reportError(EC.message() + ".\n");
 }

llvm/tools/llvm-lipo/llvm-lipo.cpp

assert(!ArchType.empty() &&
          "The architecture type should be non-empty");
   assert(InputBinaries.size() == 1 && "Incorrect number of input binaries");
   assert(!OutputFileName.empty() && "Thin expects a single output file");
 
   if (InputBinaries.front().getBinary()->isMachO()) {
     reportError("input file " +
                 InputBinaries.front().getBinary()->getFileName() +
                 " must be a fat file when the -extract option is specified");
-    exit(EXIT_FAILURE);
   }
 
   SmallVector<std::unique_ptr<MachOObjectFile>, 2> ExtractedObjects;
   SmallVector<Slice, 2> Slices =
       buildSlices(InputBinaries, Alignments, ExtractedObjects);
   erase_if(Slices, [ArchType](const Slice &S) {
     return ArchType != S.getArchString();
   });

llvm/tools/llvm-mt/llvm-mt.cpp

 };
 } // namespace
 
 LLVM_ATTRIBUTE_NORETURN void reportError(Twine Msg) {
   WithColor::error(errs(), "llvm-mt") << Msg << '\n';
   exit(1);
 }
 
-static void reportError(StringRef Input, std::error_code EC) {
+LLVM_ATTRIBUTE_NORETURN static void reportError(
+    StringRef Input, std::error_code EC) {
   reportError(Twine(Input) + ": " + EC.message());
 }
 
 void error(std::error_code EC) {
   if (EC)
     reportError(EC.message());
 }
 

llvm/tools/llvm-objdump/MachODump.cpp

             ArchitectureName = I->getArchFlagName();
           if (ObjOrErr) {
             ObjectFile &O = *ObjOrErr.get();
             if (MachOObjectFile *MachOOF = dyn_cast<MachOObjectFile>(&O))
               ProcessMachO(Filename, MachOOF, "", ArchitectureName);
           } else if (Error E = isNotObjectErrorInvalidFileType(
                          ObjOrErr.takeError())) {
             reportError(std::move(E), "", Filename, ArchitectureName);
-            continue;
           } else if (Expected<std::unique_ptr<Archive>> AOrErr =
                          I->getAsArchive()) {
             std::unique_ptr<Archive> &A = *AOrErr;
             outs() << "Archive : " << Filename;
             if (!ArchitectureName.empty())
               outs() << " (architecture " << ArchitectureName << ")";
             outs() << "\n";
             if (ArchiveHeaders)

llvm/utils/PerfectShuffle/PerfectShuffle.cpp

if (ShufTab[i].Op && !ShufTab[i].Op->isOnlyLHSOperator()) {
       } else {
         PrintMask(ShufTab[i].Arg1, std::cout);
       }
     }
     std::cout << "\n";
   }
   std::cout << "  0\n};\n";
 
-  if (0) {
+  LLVM_DEBUG({
     // Print out the table.
     for (unsigned i = 0; i != 0x8889; ++i) {
       if (!isValidMask(i)) continue;
       if (ShufTab[i].Cost < 1000) {
         PrintMask(i, std::cerr);
         std::cerr << " - Cost " << ShufTab[i].Cost << " - ";
 
         unsigned short Vals[30];
         unsigned NumVals = 0;
         EvaluateOps(i, Vals, NumVals);
 
         for (unsigned j = 0, e = NumVals; j != e; ++j)
           PrintOperation(j, Vals);
         std::cerr << "\n";
       }
     }
-  }
+  })
 }
 
 
 #ifdef GENERATE_ALTIVEC
 
 ///===---------------------------------------------------------------------===//
 /// The altivec instruction definitions.  This is the altivec-specific part of
 /// this file.

llvm/utils/TableGen/CodeGenSchedule.cpp

           assert(SchedClasses[OldSCIdx].ProcIndices[0] == 0 &&
                  "expected a generic SchedClass");
           Record *RWModelDef = InstRWDef->getValueAsDef("SchedModel");
           // Make sure we didn't already have a InstRW containing this
           // instruction on this model.
           for (Record *RWD : RWDefs) {
             if (RWD->getValueAsDef("SchedModel") == RWModelDef &&
                 RWModelDef->getValueAsBit("FullInstRWOverlapCheck")) {
-              for (Record *Inst : InstDefs) {
+              if (!InstDefs.empty()) {
                 PrintFatalError
                     (InstRWDef->getLoc(),
                      "Overlapping InstRW definition for \"" +
-                     Inst->getName() +
+                     InstDefs.first().getName() +
                      "\" also matches previous \"" +
                      RWD->getValue("Instrs")->getValue()->getAsString() +
                      "\".");
               }
             }
           }
           LLVM_DEBUG(dbgs() << "InstRW: Reuse SC " << OldSCIdx << ":"
                             << SchedClasses[OldSCIdx].Name << " on "

     SC.Writes = SchedClasses[OldSCIdx].Writes;
     SC.Reads = SchedClasses[OldSCIdx].Reads;
     SC.ProcIndices.push_back(0);
     // If we had an old class, copy it's InstRWs to this new class.
     if (OldSCIdx) {
       Record *RWModelDef = InstRWDef->getValueAsDef("SchedModel");
       for (Record *OldRWDef : SchedClasses[OldSCIdx].InstRWs) {
         if (OldRWDef->getValueAsDef("SchedModel") == RWModelDef) {
-          for (Record *InstDef : InstDefs) {
+          if (!InstDefs.empty()) {
             PrintFatalError
                 (InstRWDef->getLoc(),
                  "Overlapping InstRW definition for \"" +
-                 InstDef->getName() +
+                 InstDefs.first().getName() +
                  "\" also matches previous \"" +
                  OldRWDef->getValue("Instrs")->getValue()->getAsString() +
                  "\".");
           }
         }
         assert(OldRWDef != InstRWDef &&
                "SchedClass has duplicate InstRW def");
         SC.InstRWs.push_back(OldRWDef);