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/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
