Fix errors related to returning a CHKCBindTemporaryExprClass.

There are lots of places in the compiler that do structural matching on
a StringLiteral expression being present at a specific point in the AST.
We're now returning a CHKCBindTemporaryExprClass instead.  This changes
patches up a number of a places to skip the expression temporary.  There
are still about 26 failing Clang regression tests, though.

include/clang/AST/CanonBounds.h

@@ -10,6 +10,13 @@
 //  This file defines the interface for comparing and canonicalizing
 //  bounds expressions.
 //
+//  Bounds expressions must be non-modifying expressions, so these
+//  comparison methods should only be called on non-modifying expressions.
+//  In addition, sets of equivalent expressions should also only involve 
+//  non-modifying expressions.
+//
+//  TODO: check the invariant about expressions being non-modifying.
+//
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_CANON_BOUNDS_H
@@ -99,6 +106,8 @@ namespace clang {
     Result CompareImpl(const PositionalParameterExpr *E1,
                    const PositionalParameterExpr *E2);
     Result CompareImpl(const BoundsCastExpr *E1, const BoundsCastExpr *E2);
+    Result CompareImpl(const CHKCBindTemporaryExpr *E1, 
+                       const CHKCBindTemporaryExpr *E2);
     Result CompareImpl(const BoundsValueExpr *E1, const BoundsValueExpr *E2);
     Result CompareImpl(const AtomicExpr *E1, const AtomicExpr *E2);
     Result CompareImpl(const BlockExpr *E1, const BlockExpr *E2);

include/clang/AST/Expr.h

@@ -788,6 +788,14 @@ class Expr : public Stmt {
     return const_cast<Expr*>(this)->ignoreParenBaseCasts();
   }
 
+  /// Ignore Checked C expression temporaries (CHCKBindTemporaryExpr).
+  Expr *IgnoreExprTmp() LLVM_READONLY;
+
+  const Expr *IgnoreExprTmp() const LLVM_READONLY {
+    return const_cast<Expr*>(this)->IgnoreExprTmp();
+  }
+
+
   /// \brief Determine whether this expression is a default function argument.
   ///
   /// Default arguments are implicitly generated in the abstract syntax tree
@@ -5441,36 +5449,28 @@ class PositionalParameterExpr : public Expr {
     }
 };
 
-/// \brief Represents a temporary for use in bounds expressions.
-class BoundsTemporary {
-public:
-  static BoundsTemporary *Create(const ASTContext &C);
-};
-
-/// \brief Represents binding an expression to a temporary.
+/// \brief Represents binding an expression to an anonymous temporary.
+/// We use this binding node itself to represent the temporary.
 ///
 /// When a bounds expression is computed for an expression E, this
 /// lets the bounds expression reference the value of a subexpression
 /// of E.
+// TODO: this is very similar to an OpaqueValueExpr, except that it
+// is scoped to the top-level expression containing it.   Perhaps we
+// should combine htem.
 class CHKCBindTemporaryExpr : public Expr {
-  BoundsTemporary *Temp;
   Stmt *SubExpr;
 
 public:
-  CHKCBindTemporaryExpr(BoundsTemporary *temp, Expr* SubExpr)
+  CHKCBindTemporaryExpr(Expr* SubExpr)
    : Expr(CHKCBindTemporaryExprClass, SubExpr->getType(),
           SubExpr->getValueKind(), SubExpr->getObjectKind(), SubExpr->isTypeDependent(),
           SubExpr->isValueDependent(),
           SubExpr->isInstantiationDependent(),
-          SubExpr->containsUnexpandedParameterPack()),
-     Temp(temp), SubExpr(SubExpr) { }
+          SubExpr->containsUnexpandedParameterPack()), SubExpr(SubExpr) { }
 
   CHKCBindTemporaryExpr(EmptyShell Empty)
-    : Expr(CHKCBindTemporaryExprClass, Empty), Temp(nullptr), SubExpr(nullptr) {}
-
-  BoundsTemporary *getTemporary() { return Temp; }
-  const BoundsTemporary *getTemporary() const { return Temp; }
-  void setTemporary(BoundsTemporary *T) { Temp = T; }
+    : Expr(CHKCBindTemporaryExprClass, Empty), SubExpr(nullptr) {}
 
   const Expr *getSubExpr() const { return cast<Expr>(SubExpr); }
   Expr *getSubExpr() { return cast<Expr>(SubExpr); }

lib/AST/ASTDumper.cpp

@@ -2847,8 +2847,10 @@ void ASTDumper::VisitPositionalParameterExpr(
 
 void ASTDumper::VisitBoundsValueExpr(const BoundsValueExpr *Node) {
   VisitExpr(Node);
-  if (Node->getKind() == BoundsValueExpr::Kind::Temporary)
-    OS << " _BoundTemporary " << (unsigned int) Node->getTemporaryBinding();
+  if (Node->getKind() == BoundsValueExpr::Kind::Temporary) {
+    OS << " _BoundTemporary ";
+    dumpPointer(Node->getTemporaryBinding());
+  }
   else
     OS << " _Return_value";
 }

lib/AST/CanonBounds.cpp

@@ -321,10 +321,23 @@ Result Lexicographic::CompareExpr(const Expr *Arg1, const Expr *Arg2) {
   if (E1 == E2)
     return Result::Equal;
 
-   // Commpare expressions structurally, recursively invoking
+  // The use of an expression temporary is equal to the
+  // value of the binding expression.
+  if (BoundsValueExpr *BV1 = dyn_cast<BoundsValueExpr>(E1))
+    if (BV1->getTemporaryBinding() == E2)
+      return Result::Equal;
+
+  if (BoundsValueExpr *BV2 = dyn_cast<BoundsValueExpr>(E2))
+    if (BV2->getTemporaryBinding() == E1)
+      return Result::Equal;
+
+   // Compare expressions structurally, recursively invoking
    // comparison for subcomponents.  If that fails, consult
    // EquivExprs to see if the expressions are considered
    // equivalent.
+   // - Handle temporary variable bindings and uses specially.
+   // - Treat different kinds of casts (implicit/explicit) as
+   //   equivalent if the operation kinds are the same.
    Stmt::StmtClass E1Kind = E1->getStmtClass();
    Stmt::StmtClass E2Kind = E2->getStmtClass();
    Result Cmp = CompareInteger(E1Kind, E2Kind);
@@ -383,6 +396,9 @@ Result Lexicographic::CompareExpr(const Expr *Arg1, const Expr *Arg2) {
      case Expr::PositionalParameterExprClass: Cmp = Compare<PositionalParameterExpr>(E1, E2); break;
      case Expr::BoundsCastExprClass: Cmp = Compare<BoundsCastExpr>(E1, E2); break;
      case Expr::BoundsValueExprClass: Cmp = Compare<BoundsValueExpr>(E1, E2); break;
+     // Binding of a tempoary to the result of an expression.  These are
+     // equal if their child expressions are equal.
+     case Expr::CHKCBindTemporaryExprClass: break;
 
      // Clang extensions
      case Expr::ShuffleVectorExprClass: break;
@@ -745,15 +761,39 @@ Lexicographic::CompareImpl(const BoundsCastExpr *E1,
   return CompareType(E1->getType(), E2->getType());
 }
 
+Result
+Lexicographic::CompareImpl(const CHKCBindTemporaryExpr *E1,
+                           const CHKCBindTemporaryExpr *E2) {
+  bool ordered;                           
+  Result Cmp = ComparePointers(E1, E2, ordered);
+  if (!ordered) {
+    // Order binding expressions by the source location of
+    // the source-level expression.
+    if (E1->getSubExpr()->getLocStart() < 
+        E2->getSubExpr()->getLocStart())
+      Cmp = Result::LessThan;
+    else
+      Cmp = Result::GreaterThan;
+   }
+   return Cmp;
+}
+
 Result
 Lexicographic::CompareImpl(const BoundsValueExpr *E1,
                            const BoundsValueExpr *E2) {
   Result Cmp = CompareInteger(E1->getKind(), E2->getKind());
+  if (Cmp != Result::Equal)
+    return Cmp;
+
   // The type doesn't matter - the value is the same no matter what the type.
+
+  // If these are uses of an expression temporary, check that the binding
+  // expression is the same.
+  if (E1->getKind() == BoundsValueExpr::Kind::Temporary)
+     Cmp = CompareImpl(E1->getTemporaryBinding(), E2->getTemporaryBinding());
   return Cmp;
 }
 
-
 Result
 Lexicographic::CompareImpl(const BlockExpr *E1, const BlockExpr *E2) {
   return Result::Equal;

lib/AST/Expr.cpp

@@ -1668,6 +1668,9 @@ namespace {
     if (CXXBindTemporaryExpr *Binder = dyn_cast<CXXBindTemporaryExpr>(expr))
       expr = Binder->getSubExpr();
 
+    if (CHKCBindTemporaryExpr *Temp = dyn_cast<CHKCBindTemporaryExpr>(expr))
+      expr = Temp->getSubExpr();
+
     return expr;
   }
 }
@@ -1963,7 +1966,7 @@ bool InitListExpr::isStringLiteralInit() const {
   const Expr *Init = getInit(0);
   if (!Init)
     return false;
-  Init = Init->IgnoreParens();
+  Init = Init->IgnoreParens()->IgnoreExprTmp();
   return isa<StringLiteral>(Init) || isa<ObjCEncodeExpr>(Init);
 }
 
@@ -2010,15 +2013,15 @@ bool InitListExpr::isNullTerminated(ASTContext &C, unsigned DeclArraySize) const
     return true;
   }
 
-  if (getNumInits() == 1 && getInit(0) && isa<StringLiteral>(getInit(0))) {
-    const StringLiteral *InitializerString = cast<StringLiteral>(getInit(0));
-    if (DeclArraySize < InitializerString->getLength())
-    {
-      const char *StringConstant = InitializerString->getString().data();
-      return (StringConstant[DeclArraySize - 1] == '\0');  
+  if (getNumInits() == 1 && getInit(0))
+    if (const StringLiteral *InitializerString =
+         dyn_cast<StringLiteral>(getInit(0)->IgnoreExprTmp())) {
+      if (DeclArraySize < InitializerString->getLength()) {
+        const char *StringConstant = InitializerString->getString().data();
+        return (StringConstant[DeclArraySize - 1] == '\0');
+      }
+      return true;
     }
-    return true;
-  }
 
   const Expr *LastItem = getInit(getNumInits() - 1);
   Expr::NullPointerConstantKind E = LastItem->isNullPointerConstant(C, Expr::NPC_ValueDependentIsNull);
@@ -2415,6 +2418,9 @@ bool Expr::isUnusedResultAWarning(const Expr *&WarnE, SourceLocation &Loc,
   case CXXBindTemporaryExprClass:
     return cast<CXXBindTemporaryExpr>(this)->getSubExpr()
                ->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+  case CHKCBindTemporaryExprClass:
+    return cast<CHKCBindTemporaryExpr>(this)->getSubExpr()
+               ->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
   case ExprWithCleanupsClass:
     return cast<ExprWithCleanups>(this)->getSubExpr()
                ->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
@@ -2541,6 +2547,10 @@ Expr *Expr::IgnoreParenCasts() {
       E = NTTP->getReplacement();
       continue;
     }      
+    if (CHKCBindTemporaryExpr *Binding = dyn_cast<CHKCBindTemporaryExpr>(E)) {
+      E = Binding->getSubExpr();
+      continue;
+    }
     return E;
   }
 }
@@ -2562,6 +2572,10 @@ Expr *Expr::IgnoreCasts() {
       E = NTTP->getReplacement();
       continue;
     }
+    if (CHKCBindTemporaryExpr *Binding = dyn_cast<CHKCBindTemporaryExpr>(E)) {
+      E = Binding->getSubExpr();
+      continue;
+    }
     return E;
   }
 }
@@ -2588,6 +2602,10 @@ Expr *Expr::IgnoreParenLValueCasts() {
       E = NTTP->getReplacement();
       continue;
     }
+    if (CHKCBindTemporaryExpr *Binding = dyn_cast<CHKCBindTemporaryExpr>(E)) {
+      E = Binding->getSubExpr();
+      continue;
+    }
     break;
   }
   return E;
@@ -2628,6 +2646,10 @@ Expr *Expr::IgnoreParenImpCasts() {
       E = NTTP->getReplacement();
       continue;
     }
+    if (CHKCBindTemporaryExpr *Binding = dyn_cast<CHKCBindTemporaryExpr>(E)) {
+      E = Binding->getSubExpr();
+      continue;
+    }
     return E;
   }
 }
@@ -2678,6 +2700,14 @@ Expr *Expr::IgnoreParenNoopCasts(ASTContext &Ctx) {
   }
 }
 
+Expr *Expr::IgnoreExprTmp() {
+  Expr *E = this;
+  if (CHKCBindTemporaryExpr *Binding = dyn_cast<CHKCBindTemporaryExpr>(E))
+    return Binding->getSubExpr();
+
+  return E;
+}
+
 bool Expr::isDefaultArgument() const {
   const Expr *E = this;
   if (const MaterializeTemporaryExpr *M = dyn_cast<MaterializeTemporaryExpr>(E))
@@ -2705,6 +2735,9 @@ static const Expr *skipTemporaryBindingsNoOpCastsAndParens(const Expr *E) {
   while (const CXXBindTemporaryExpr *BE = dyn_cast<CXXBindTemporaryExpr>(E))
     E = BE->getSubExpr();
 
+  while (const CHKCBindTemporaryExpr *CB = dyn_cast<CHKCBindTemporaryExpr>(E))
+    E = CB->getSubExpr();
+
   while (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) {
     if (ICE->getCastKind() == CK_NoOp)
       E = ICE->getSubExpr();
@@ -2967,7 +3000,11 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef,
   case CXXDefaultInitExprClass:
     return cast<CXXDefaultInitExpr>(this)->getExpr()
       ->isConstantInitializer(Ctx, false, Culprit);
+  case CHKCBindTemporaryExprClass:
+    return cast<CHKCBindTemporaryExpr>(this)->getSubExpr()
+      ->isConstantInitializer(Ctx, false, Culprit);
   }
+
   // Allow certain forms of UB in constant initializers: signed integer
   // overflow and floating-point division by zero. We'll give a warning on
   // these, but they're common enough that we have to accept them.
@@ -3071,10 +3108,6 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
   case BoundsValueExprClass:
     // These never have a side-effect.
     return false;
-  case CHKCBindTemporaryExprClass:
-    // These have a side-effect if the subexpression has 
-    // a side-effect.
-    break;
 
   case CallExprClass:
   case CXXOperatorCallExprClass:
@@ -3096,6 +3129,9 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
     if (!IncludePossibleEffects)
       break;
     return true;
+  case CHKCBindTemporaryExprClass:
+    // These have a side-effect if the subexpression has a side-effect.
+    break;
 
   case MSPropertyRefExprClass:
   case MSPropertySubscriptExprClass:

lib/AST/ExprConstant.cpp

@@ -5137,6 +5137,8 @@ class LValueExprEvaluator
       return HandleBaseToDerivedCast(Info, E, Result);
     }
   }
+
+  bool VisitCHKCBindTemporaryExpr(const CHKCBindTemporaryExpr *E);
 };
 } // end anonymous namespace
 
@@ -5428,6 +5430,11 @@ bool LValueExprEvaluator::VisitBinAssign(const BinaryOperator *E) {
                           NewVal);
 }
 
+bool LValueExprEvaluator::VisitCHKCBindTemporaryExpr(
+  const CHKCBindTemporaryExpr *E) {
+  return Visit(E->getSubExpr());
+}
+
 //===----------------------------------------------------------------------===//
 // Pointer Evaluation
 //===----------------------------------------------------------------------===//

lib/Analysis/ThreadSafetyCommon.cpp

@@ -147,7 +147,7 @@ CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp,
   if (!AttrExp)
     return CapabilityExpr(nullptr, false);
 
-  if (auto* SLit = dyn_cast<StringLiteral>(AttrExp)) {
+  if (auto* SLit = dyn_cast<StringLiteral>(AttrExp->IgnoreExprTmp())) {
     if (SLit->getString() == StringRef("*"))
       // The "*" expr is a universal lock, which essentially turns off
       // checks until it is removed from the lockset.

lib/CodeGen/CGExprConstant.cpp

@@ -833,6 +833,11 @@ class ConstExprEmitter :
     return Visit(E->GetTemporaryExpr(), T);
   }
 
+  llvm::Constant *VisitCHKCBindTemporaryExpr(CHKCBindTemporaryExpr *E,
+                                             QualType T) {
+    return Visit(E->getSubExpr(), T);
+  }
+
   llvm::Constant *EmitArrayInitialization(InitListExpr *ILE, QualType T) {
     llvm::ArrayType *AType =
         cast<llvm::ArrayType>(ConvertType(ILE->getType()));