[Clang]Ensure correct handling of cleanup access control (#135668) (#135686)

The P2719 implementation refactored diagnostics for cleanup delete, and
as part of that I attempted to fix handling of inaccessible cleanup
operator delete. Alas the new branch was incorrect as it was performing
an implicit bool conversion, which resulted in friend accessible cleanup
operators incorrectly being considered erroneous and the allocation path
errored out. This error however did not get diagnosed, so the result was
and so we did not actually error out before codegen.

Added both Sema and CodeGen tests to cover this.

Author: ojhunt

clang/lib/Sema/SemaExprCXX.cpp

@@ -1929,8 +1929,9 @@ static bool CheckDeleteOperator(Sema &S, SourceLocation StartLoc,
     }
     return true;
   }
-
-  return S.CheckAllocationAccess(StartLoc, Range, NamingClass, Decl, Diagnose);
+  Sema::AccessResult Accessible =
+      S.CheckAllocationAccess(StartLoc, Range, NamingClass, Decl, Diagnose);
+  return Accessible == Sema::AR_inaccessible;
 }
 
 /// Select the correct "usual" deallocation function to use from a selection of

clang/test/CodeGenCXX/bug135668.cpp

@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 %s -triple arm64-apple-macosx -emit-llvm -fcxx-exceptions -fexceptions -std=c++23 -o - | FileCheck  %s
+
+class TestClass {
+  public:
+   TestClass();
+   int field = 0;
+   friend class Foo;
+   static void * operator new(unsigned long size);
+  private:
+   static void operator delete(void *p);
+ };
+
+class Foo {
+public:
+  int test_method();
+};
+
+int Foo::test_method() {
+  TestClass *obj = new TestClass() ;
+  return obj->field;
+}
+
+// CHECK-LABEL: define noundef i32 @_ZN3Foo11test_methodEv
+// CHECK: [[THIS_ADDR:%.*]] = alloca ptr, align 8
+// CHECK: [[OBJ:%.*]] = alloca ptr, align 8
+// CHECK: store ptr %this, ptr [[THIS_ADDR]], align 8
+// CHECK: [[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8
+// CHECK: [[ALLOCATION:%.*]] = call noundef ptr @_ZN9TestClassnwEm(i64 noundef 4)
+// CHECK: [[INITIALIZEDOBJ:%.*]] = invoke noundef ptr @_ZN9TestClassC1Ev(ptr noundef nonnull align 4 dereferenceable(4) [[ALLOCATION]])
+// CHECK-NEXT:  to label %[[INVOKE_CONT:.*]] unwind label %[[LPAD:.*]]
+// CHECK: [[INVOKE_CONT]]:
+// CHECK: store ptr [[ALLOCATION]], ptr [[OBJ]], align 8
+// CHECK: [[OBJPTR:%.*]] = load ptr, ptr [[OBJ]], align 8
+// CHECK: [[FIELDPTR:%.*]] = getelementptr inbounds nuw %class.TestClass, ptr [[OBJPTR]], i32 0, i32 0
+// CHECK: [[FIELD:%.*]] = load i32, ptr [[FIELDPTR]], align 4
+// CHECK: ret i32 [[FIELD]]
+// CHECK: [[LPAD]]:
+// CHECK: call void @_ZN9TestClassdlEPv(ptr noundef [[ALLOCATION]]) #3

clang/test/SemaCXX/bug135668.cpp

@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple arm64-apple-macosx -Wall -fsyntax-only -verify %s -std=c++26 -fexceptions -fcxx-exceptions
+// expected-no-diagnostics
+
+// This test makes sure that we don't erroneously consider an accessible operator
+// delete to be inaccessible, and then discard the entire new expression.
+
+class TestClass {
+public:
+  TestClass();
+  int field = 0;
+  friend class Foo;
+  static void * operator new(unsigned long size);
+private:
+  static void operator delete(void *p);
+};
+
+class Foo {
+public:
+  int test_method();
+};
+
+int Foo::test_method() {
+  TestClass *obj = new TestClass() ;
+  return obj->field;
+}