Compare Expressions: Fix the conditions for coalescing a node with its parent

clang/include/clang/AST/PreorderAST.h

@@ -88,11 +88,20 @@ namespace clang {
     // @param[in] Parent is the parent of the node to be added.
     void AddNode(Node *N, Node *Parent);
 
-    // Move the children (if any) of node N to its parent and then remove N.
-    // @param[in] N is the current node.
+    // Coalesce the BinaryNode B with its parent P. This involves moving the
+    // children (if any) of node B to its parent and then removing B.
+    // @param[in] B is the current node. B should be a BinaryNode.
     // @param[in] P is the parent of the node to be removed. P should be a
     // BinaryNode.
-    void RemoveNode(Node *N, Node *P);
+    void CoalesceNode(BinaryNode *B, BinaryNode *P);
+
+    // Determines if a BinaryNode could be coalesced into its parent.
+    // @param[in] B is the current node. B should be a BinaryNode.
+    // @param[in] P is the parent of the node to be removed. P should be a
+    // BinaryNode.
+    // @return Return true if B can be coalesced into its parent P, false
+    // otherwise.
+    bool CanCoalesceNode(BinaryNode *B, BinaryNode *P);
 
     // Recursively coalesce binary nodes having the same commutative and
     // associative operator.

clang/lib/AST/PreorderAST.cpp

@@ -29,51 +29,48 @@ void PreorderAST::AddNode(Node *N, Node *Parent) {
   }
 }
 
-void PreorderAST::RemoveNode(Node *N, Node *Parent) {
-  // The parent should be a BinaryNode.
-  assert(isa<BinaryNode>(Parent) && "Invalid parent");
+bool PreorderAST::CanCoalesceNode(BinaryNode *B, BinaryNode *P) {
+  if (!B || !isa<BinaryNode>(B) ||
+      !P || !isa<BinaryNode>(P))
+    return false;
+
+  // We can coalesce a BinaryNode if any one of the following conditions are
+  // true:
+  // 1. The parent has the same operator as the current node OR
+  // 2. The current node has just one child (for example, as a result of
+  // constant folding) and the parent and current operators are commutative and
+  // associative.
+
+  return  B->Opc == P->Opc ||
+         (B->Children.size() == 1 &&
+          B->IsOpCommutativeAndAssociative() &&
+          P->IsOpCommutativeAndAssociative());
+}
 
-  auto *P = dyn_cast<BinaryNode>(Parent);
-  if (!P) {
+void PreorderAST::CoalesceNode(BinaryNode *B, BinaryNode *P) {
+  if (!CanCoalesceNode(B, P)) {
+    assert(0 && "Attempting to coalesce invalid node");
     SetError();
     return;
   }
 
-  // We will remove a BinaryNode only if its operator is equal to its
-  // parent's operator and the operator is commutative and associative.
-  if (auto *B = dyn_cast<BinaryNode>(N)) {
-    assert(B->Opc == P->Opc &&
-           "BinaryNode operator must equal parent operator");
-
-    assert(B->IsOpCommutativeAndAssociative() &&
-           "BinaryNode operator must be commutative and associative");
-
-    if (B->Opc != P->Opc ||
-       !B->IsOpCommutativeAndAssociative()) {
-      SetError();
-      return;
-    }
-  }
-
   // Remove the current node from the list of children of its parent.
   for (auto I = P->Children.begin(),
             E = P->Children.end(); I != E; ++I) {
-    if (*I == N) {
+    if (*I == B) {
       P->Children.erase(I);
       break;
     }
   }
 
-  if (auto *B = dyn_cast<BinaryNode>(N)) {
-    // Move all children of the current node to its parent.
-    for (auto *Child : B->Children) {
-      Child->Parent = P;
-      P->Children.push_back(Child);
-    }
+  // Move all children of the current node to its parent.
+  for (auto *Child : B->Children) {
+    Child->Parent = P;
+    P->Children.push_back(Child);
   }
 
   // Delete the current node.
-  delete N;
+  delete B;
 }
 
 void PreorderAST::Create(Expr *E, Node *Parent) {
@@ -170,12 +167,8 @@ void PreorderAST::Coalesce(Node *N, bool &Changed) {
   if (!Parent)
     return;
 
-  // We can coalesce only if:
-  // 1. The parent has the same operator as the current node.
-  // 2. The current node is a BinaryNode with just one child (for example, as a
-  // result of constant folding).
-  if (Parent->Opc == B->Opc || B->Children.size() == 1) {
-    RemoveNode(B, Parent);
+  if (CanCoalesceNode(B, Parent)) {
+    CoalesceNode(B, Parent);
     Changed = true;
   }
 }

clang/test/CheckedC/inferred-bounds/widened-bounds-semantic-compare.c

@@ -169,3 +169,15 @@ void f14(int i) {
 // CHECK:  [B1]
 // CHECK-NOT: upper_bound(p)
 }
+
+void f15(_Nt_array_ptr<char> p : count(6)) {
+  if (*(p + (1 * (2 * 3))))
+  {}
+
+// CHECK: In function: f15
+//  [B2]
+//    1: _Nt_array_ptr<char> p : count(6) = "a";
+//    2: *(p + (1 * (2 * 3)))
+//  [B1]
+// upper_bound(p) = 1
+}