[BoundsSafety] Do not merge param/return types if there is no need to

If pointers don't have any interesting attributes, there is no need to
attempt to merge the types, and we can keep the type unchanged. This
will let us keep the sugars used in the new declaration.

rdar://153579566

Author: patrykstefanski

clang/lib/AST/ASTContext.cpp

@@ -3932,21 +3932,6 @@ QualType ASTContext::mergeBoundsSafetyPointerTypes(
   if (OrigDstTy.isNull())
     OrigDstTy = DstTy;
 
-  // An ugly way to keep va_list typedef in DstTy if the merge type doesn't
-  // change.
-  // TODO: We need a general way of not stripping sugars.
-  QualType DesugaredDstTy;
-  if (const auto *TDT = dyn_cast<TypedefType>(DstTy))
-    DesugaredDstTy = TDT->desugar();
-  else if (const auto *ET = dyn_cast<ElaboratedType>(DstTy))
-    DesugaredDstTy = ET->desugar();
-  if (!DesugaredDstTy.isNull()) {
-    QualType MergeTy = mergeBoundsSafetyPointerTypes(DesugaredDstTy, SrcTy,
-                                                     MergeFunctor, OrigDstTy);
-    if (MergeTy == DesugaredDstTy)
-      return DstTy;
-  }
-
   // FIXME: a brittle hack to avoid skipping ValueTerminatedType outside
   // this PtrAutoAttr AttributedType.
   bool RecoverPtrAuto = false;

clang/lib/Sema/SemaDecl.cpp

@@ -4100,12 +4100,33 @@ static bool mergeFunctionDeclBoundsAttributes(FunctionDecl *New,
     }
     return MergeTy;
   };
+
+  auto hasBoundsAttributesAtAnyLevel = [](QualType Ty) -> bool {
+    while (Ty->isPointerType()) {
+      if (!Ty->isUnspecifiedPointerType())
+        return true;
+      Ty = Ty->getPointeeType();
+    }
+    return false;
+  };
+
+  auto mergePointersWithAttributes = [&](QualType NewTy,
+                                         QualType OldTy) -> QualType {
+    // Do not attempt to merge if there is no need to do so. This will let us
+    // keep the sugars used in the new declaration.
+    if (!hasBoundsAttributesAtAnyLevel(NewTy) &&
+        !hasBoundsAttributesAtAnyLevel(OldTy))
+      return NewTy;
+    return Self.Context.mergeBoundsSafetyPointerTypes(NewTy, OldTy,
+                                                      mergeBoundsAttributes);
+  };
+
   QualType NewRetTy = New->getReturnType();
   if (NewRetTy->isBoundsAttributedType() ||
       NewRetTy->isValueTerminatedType())
     return false;
-  QualType MergeRetTy = Self.Context.mergeBoundsSafetyPointerTypes(
-      New->getReturnType(), Old->getReturnType(), mergeBoundsAttributes);
+  QualType MergeRetTy =
+      mergePointersWithAttributes(New->getReturnType(), Old->getReturnType());
   if (MergeRetTy.isNull())
     return false;
 
@@ -4115,9 +4136,8 @@ static bool mergeFunctionDeclBoundsAttributes(FunctionDecl *New,
     if (NewParmTy->isBoundsAttributedType() ||
         NewParmTy->isValueTerminatedType())
       return false;
-    QualType MergeParamTy = Self.Context.mergeBoundsSafetyPointerTypes(
-        NewParmTy, Old->getParamDecl(i)->getType(),
-        mergeBoundsAttributes);
+    QualType MergeParamTy =
+        mergePointersWithAttributes(NewParmTy, Old->getParamDecl(i)->getType());
     if (MergeParamTy.isNull())
       return false;
     if (const auto *CATy = MergeParamTy->getAs<CountAttributedType>()) {

clang/test/BoundsSafety/AST/system-header-merge-bounds-attributed.c

@@ -29,6 +29,9 @@
 // CHECK: FunctionDecl {{.+}} eb
 // CHECK: |-ParmVarDecl {{.+}} eb_p 'void *'
 // CHECK: `-ParmVarDecl {{.+}} end 'void *'
+// CHECK: FunctionDecl {{.+}} cb_array
+// CHECK: |-ParmVarDecl {{.+}} array 'int *'
+// CHECK: `-ParmVarDecl {{.+}} len 'int'
 
 
 // Check if we can override them.
@@ -39,6 +42,7 @@ void cb_out_count(int *__counted_by(*len) cb_out_len_p, int *len);
 void cbn(int *__counted_by_or_null(len) cbn_p, int len);
 void sb(void *__sized_by(size) sb_p, int size);
 void eb(void *__ended_by(end) eb_p, void *end);
+void cb_array(int array[__counted_by(len)], int len);
 
 // CHECK: FunctionDecl {{.+}} prev {{.+}} cb_in
 // CHECK: |-ParmVarDecl {{.+}} cb_in_p 'int *{{.*}} __counted_by(len)'
@@ -58,6 +62,9 @@ void eb(void *__ended_by(end) eb_p, void *end);
 // CHECK: FunctionDecl {{.+}} prev {{.+}} eb
 // CHECK: |-ParmVarDecl {{.+}} used eb_p 'void *{{.*}} __ended_by(end)'
 // CHECK: `-ParmVarDecl {{.+}} used end 'void *{{.*}} /* __started_by(eb_p) */ '
+// CHECK: FunctionDecl {{.+}} prev {{.+}} cb_array
+// CHECK: |-ParmVarDecl {{.+}} array 'int *{{.*}} __counted_by(len)'
+// CHECK: `-ParmVarDecl {{.+}} used len 'int'
 
 
 // Check if the attributes are merged.
@@ -82,3 +89,6 @@ void eb(void *__ended_by(end) eb_p, void *end);
 // CHECK: FunctionDecl {{.+}} prev {{.+}} eb
 // CHECK: |-ParmVarDecl {{.+}} used eb_p 'void *{{.*}} __ended_by(end)'
 // CHECK: `-ParmVarDecl {{.+}} used end 'void *{{.*}} /* __started_by(eb_p) */ '
+// CHECK: FunctionDecl {{.+}} prev {{.+}} cb_array
+// CHECK: |-ParmVarDecl {{.+}} array 'int *{{.*}} __counted_by(len)'
+// CHECK: `-ParmVarDecl {{.+}} used len 'int'

clang/test/BoundsSafety/AST/system-header-merge-bounds-attributed.h

@@ -6,3 +6,4 @@ void cb_out_count(int *cb_out_len_p, int *len);
 void cbn(int *cbn_p, int len);
 void sb(void *sb_p, int size);
 void eb(void *eb_p, void *end);
+void cb_array(int array[], int len);