[PATCH] Refactor: Simplify boolean conditional return statements in lib/Sema

[David Blaikie]

Got only part way through this one.

We should come to some conclusion about these chained return cases - they seem fairly benign/not buggy/not worth changing.

Basically what I mean when I say "chain" or "possible chain" is that it looks like this last if/return is just another case of several in the same function and shouldn't be treated differently by being rolled into one final return statement. Mostly the question I think that's worth asking is "could I reasonably add another condition on to the end of this function?" and if so, it hurts readability/writability by rolling it in and making it somehow different from the other cases in the function.

I think this was a concern John McCall raised a few weeks ago on the same sort of changes you'd been proposing.


================
Comment at: lib/Sema/SemaChecking.cpp:4627
@@ -4626,6 +4626,3 @@
 
-  if (!isa<TranslationUnitDecl>(ND->getDeclContext()))
-    return false;
-
-  return true;
+  return isa<TranslationUnitDecl>(ND->getDeclContext());
 }
----------------
Possible chain

================
Comment at: lib/Sema/SemaChecking.cpp:8687
@@ -8692,6 +8686,3 @@
 
-  if (checkUnsafeAssignObject(*this, Loc, LT, RHS, false))
-    return true;
-
-  return false;
+  return checkUnsafeAssignObject(*this, Loc, LT, RHS, false);
 }
----------------
Possible chain

================
Comment at: lib/Sema/SemaChecking.cpp:8782
@@ -8790,6 +8781,3 @@
   // Warn if null statement and body are on the same line.
-  if (StmtLine != BodyLine)
-    return false;
-
-  return true;
+  return StmtLine == BodyLine;
 }
----------------
Possible chain

================
Comment at: lib/Sema/SemaDecl.cpp:2531
@@ -2530,5 +2530,3 @@
     return true;
-  if (OldLinkage == CLanguageLinkage && New->isInExternCXXContext())
-    return true;
-  return false;
+  return OldLinkage == CLanguageLinkage && New->isInExternCXXContext();
 }
----------------
Possible chain

================
Comment at: lib/Sema/SemaDecl.cpp:13607
@@ -13610,6 +13606,3 @@
 
-  if (cast<EnumDecl>(TagDecl::castFromDeclContext(ECD->getDeclContext())) !=
-      Enum)
-    return true;
-
-  return false;
+  return cast<EnumDecl>(TagDecl::castFromDeclContext(ECD->getDeclContext())) !=
+         Enum;
----------------
Possible chain

================
Comment at: lib/Sema/SemaDeclAttr.cpp:366
@@ -365,6 +365,3 @@
       S.Context.DeclarationNames.getCXXOperatorName(OO_Arrow));
-  if (Res2.empty())
-    return false;
-
-  return true;
+  return !Res2.empty();
 }
----------------
possible chain

================
Comment at: lib/Sema/SemaDeclAttr.cpp:457
@@ -459,6 +456,3 @@
 
-  if (checkRecordTypeForCapability(S, Ty))
-    return true;
-
-  return false;
+  return checkRecordTypeForCapability(S, Ty);
 }
----------------
Possible chain

================
Comment at: lib/Sema/SemaDeclAttr.cpp:1805
@@ -1813,6 +1804,3 @@
 
-  if (BeforeIsOkay && X < Y)
-    return true;
-
-  return false;
+  return BeforeIsOkay && X < Y;
 }
----------------
possible chain

http://reviews.llvm.org/D10019

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/