r203091 - [-Wunreachable-code] don't warn about dead 'return <string literal>' dominated by a 'noreturn' call, where literal becomes an std::string.

[Ted Kremenek]

Author: kremenek
Date: Thu Mar  6 00:50:46 2014
New Revision: 203091

URL: http://llvm.org/viewvc/llvm-project?rev=203091&view=rev
Log:
[-Wunreachable-code] don't warn about dead 'return <string literal>' dominated by a 'noreturn' call, where literal becomes an std::string.

I have mixed feelings about this one.  It's used all over the codebase,
and is analogous to the current heuristic for ordinary C string literals.

This requires some ad hoc pattern matching of the AST.  While the
test case mirrors what we see std::string in libc++, it's not really
testing the libc++ headers.

Modified:
    cfe/trunk/lib/Analysis/ReachableCode.cpp
    cfe/trunk/test/SemaCXX/warn-unreachable.cpp

Modified: cfe/trunk/lib/Analysis/ReachableCode.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ReachableCode.cpp?rev=203091&r1=203090&r2=203091&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ReachableCode.cpp (original)
+++ cfe/trunk/lib/Analysis/ReachableCode.cpp Thu Mar  6 00:50:46 2014
@@ -251,7 +251,10 @@ static bool bodyEndsWithNoReturn(const C
   for (CFGBlock::const_reverse_iterator I = B->rbegin(), E = B->rend();
        I != E; ++I) {
     if (Optional<CFGStmt> CS = I->getAs<CFGStmt>()) {
-      if (const CallExpr *CE = dyn_cast<CallExpr>(CS->getStmt())) {
+      const Stmt *S = CS->getStmt();
+      if (const ExprWithCleanups *EWC = dyn_cast<ExprWithCleanups>(S))
+        S = EWC->getSubExpr();
+      if (const CallExpr *CE = dyn_cast<CallExpr>(S)) {
         QualType CalleeType = CE->getCallee()->getType();
         if (getFunctionExtInfo(*CalleeType).getNoReturn())
           return true;
@@ -290,6 +293,53 @@ static bool isEnumConstant(const Expr *E
   return isa<EnumConstantDecl>(DR->getDecl());
 }
 
+static const Expr *stripStdStringCtor(const Expr *Ex) {
+  // Go crazy pattern matching an implicit construction of std::string("").
+  const ExprWithCleanups *EWC = dyn_cast<ExprWithCleanups>(Ex);
+  if (!EWC)
+    return 0;
+  const CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(EWC->getSubExpr());
+  if (!CCE)
+    return 0;
+  QualType Ty = CCE->getType();
+  if (const ElaboratedType *ET = dyn_cast<ElaboratedType>(Ty))
+    Ty = ET->getNamedType();
+  const TypedefType *TT = dyn_cast<TypedefType>(Ty);
+  StringRef Name = TT->getDecl()->getName();
+  if (Name != "string")
+    return 0;
+  if (CCE->getNumArgs() != 1)
+    return 0;
+  const MaterializeTemporaryExpr *MTE =
+    dyn_cast<MaterializeTemporaryExpr>(CCE->getArg(0));
+  if (!MTE)
+    return 0;
+  CXXBindTemporaryExpr *CBT =
+    dyn_cast<CXXBindTemporaryExpr>(MTE->GetTemporaryExpr()->IgnoreParenCasts());
+  if (!CBT)
+    return 0;
+  Ex = CBT->getSubExpr()->IgnoreParenCasts();
+  CCE = dyn_cast<CXXConstructExpr>(Ex);
+  if (!CCE)
+    return 0;
+  if (CCE->getNumArgs() != 1)
+    return 0;
+  return dyn_cast<StringLiteral>(CCE->getArg(0)->IgnoreParenCasts());
+}
+
+/// Strip away "sugar" around trivial expressions that are for the
+/// purpose of this analysis considered uninteresting for dead code warnings.
+static const Expr *stripExprSugar(const Expr *Ex) {
+  Ex = Ex->IgnoreParenCasts();
+  // If 'Ex' is a constructor for a std::string, strip that
+  // away.  We can only get here if the trivial expression was
+  // something like a C string literal, with the std::string
+  // just wrapping that value.
+  if (const Expr *StdStringVal = stripStdStringCtor(Ex))
+    return StdStringVal;
+  return Ex;
+}
+
 static bool isTrivialExpression(const Expr *Ex) {
   Ex = Ex->IgnoreParenCasts();
   return isa<IntegerLiteral>(Ex) || isa<StringLiteral>(Ex) ||
@@ -324,7 +374,7 @@ static bool isTrivialReturnOrDoWhile(con
     if (Optional<CFGStmt> CS = I->getAs<CFGStmt>()) {
       if (const ReturnStmt *RS = dyn_cast<ReturnStmt>(CS->getStmt())) {
         const Expr *RE = RS->getRetValue();
-        if (RE && RE->IgnoreParenCasts() == Ex)
+        if (RE && stripExprSugar(RE->IgnoreParenCasts()) == Ex)
           return bodyEndsWithNoReturn(*B->pred_begin());
       }
       break;

Modified: cfe/trunk/test/SemaCXX/warn-unreachable.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unreachable.cpp?rev=203091&r1=203090&r2=203091&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-unreachable.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-unreachable.cpp Thu Mar  6 00:50:46 2014
@@ -129,4 +129,25 @@ PR19040_test_return_t PR19040_fn1 ()
     return PR19040_TEST_FAILURE; // expected-warning {{will never be executed}}
 }
 
+__attribute__((noreturn))
+void raze();
+
+namespace std {
+template<typename T> struct basic_string {
+  basic_string(const T* x) {}
+  ~basic_string() {};
+};
+typedef basic_string<char> string;
+}
+
+std::string testStr() {
+  raze();
+  return ""; // no-warning
+}
+
+std::string testStrWarn(const char *s) {
+  raze();
+  return s; // expected-warning {{will never be executed}}
+}
+