[cfe-commits] [PATCH] Weird code cleanup.

[Douglas Gregor]

On Nov 1, 2011, at 9:22 PM, Ahmed Charles wrote:

> I was going through some unreachable code warnings and noticed various
> instances of either stale code or typo's or logic errors. Here's a
> patch that attempts to fix them, which I'm confident it does for most
> of them, but one or two probably require a closer look by people more
> familiar with the code (like the mips target validation code).
> 
> Anyways, enjoy.
> <0002-Weird-code-that-is-probably-not-correct.-Propose-fix.patch>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

diff --git a/lib/Basic/Targets.cpp b/lib/Basic/Targets.cpp
index da32c93..2463365 100644
--- a/lib/Basic/Targets.cpp
+++ b/lib/Basic/Targets.cpp
@@ -3076,7 +3076,7 @@ public:
   virtual bool validateAsmConstraint(const char *&Name,
                                      TargetInfo::ConstraintInfo &Info) const {
     switch (*Name) {
-    default:
+    default: assert(0 && "validateAsmConstraint thinks anything is valid");
     case 'r': // CPU registers.
     case 'd': // Equivalent to "r" unless generating MIPS16 code.
     case 'y': // Equivalent to "r", backwards compatibility only.
@@ -3084,7 +3084,6 @@ public:
       Info.setAllowsRegister();
       return true;
     }
-    return false;
   }
 
   virtual const char *getClobbers() const {

I don't think we want to assert here; just return false (I've fixed this in the commit).

index 5c5ddca..d304a09 100644
--- a/lib/Frontend/CompilerInvocation.cpp
+++ b/lib/Frontend/CompilerInvocation.cpp
@@ -707,10 +707,6 @@ static void LangOptsToArgs(const LangOptions &Opts,
   case LangOptions::SOB_Defined:   Res.push_back("-fwrapv"); break;
   case LangOptions::SOB_Trapping:
     Res.push_back("-ftrapv"); break;
-    if (!Opts.OverflowHandler.empty()) {
-      Res.push_back("-ftrapv-handler");
-      Res.push_back(Opts.OverflowHandler);
-    }
   }
   if (Opts.HeinousExtensions)
     Res.push_back("-fheinous-gnu-extensions");

It looks like the break was just in the wrong place.

--- a/lib/Driver/OptTable.cpp
+++ b/lib/Driver/OptTable.cpp
@@ -188,7 +188,6 @@ Option *OptTable::CreateOption(unsigned id) const {
 // Index is just a padding argument, no real meaning...
 Arg *OptTable::ParseOneArgTrap(const ArgList &Args, unsigned &Index,
                                const char *Str) const {
-  unsigned Prev = Index;
   // Anything that doesn't start with '-' is an input, as is '-' itself.
   if (Str[0] != '-' || Str[1] == '\0')
     return new Arg(TheInputOption, Index++, Str);
@@ -199,21 +198,16 @@ Arg *OptTable::ParseOneArgTrap(const ArgList &Args, unsigned &Index,
   // Search for the first next option which could be a prefix.
   Start = std::lower_bound(Start, End, Str);
 
-  for (; Start != End; ++Start) {
+  if (Start != End) {
     // Scan for first option which is a proper prefix.
     for (; Start != End; ++Start)
       if (memcmp(Str, Start->Name, strlen(Start->Name)) == 0)
         break;
-    if (Start == End)
-      break;
-
-    // -lxxx, just tease out 'xxx' part...
-    const char *strPart = Str + 2;
-    return new Arg(getOption(Start - OptionInfos + 1), Index++, strPart);
-
-    // Otherwise, see if this argument was missing values.
-    if (Prev != Index)
-      return 0;
+    if (Start != End) {
+      // -lxxx, just tease out 'xxx' part...
+      const char *strPart = Str + 2;
+      return new Arg(getOption(Start - OptionInfos + 1), Index++, strPart);
+    }
   }
 
   return new Arg(TheUnknownOption, Index++, Str);

This big doesn't apply to top-of-tree Clang, where the Prev != Index checking actually makes a bit more sense.

Committed as r143569, thanks!

	- Doug