Skip to content

Conversation

@philnik777
Copy link
Contributor

This aligns the builtins with how implementations work which don't use
the buitins.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 31, 2025
@philnik777 philnik777 added this to the LLVM 20.X Release milestone Jan 31, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 31, 2025

@llvm/pr-subscribers-clang

Author: Nikolas Klauser (philnik777)

Changes

This aligns the builtins with how implementations work which don't use
the buitins.


Full diff: https://github.com/llvm/llvm-project/pull/125185.diff

4 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/lib/Sema/SemaType.cpp (+3-3)
  • (removed) clang/test/SemaCXX/remove_pointer.mm (-8)
  • (added) clang/test/SemaObjCXX/type-traits.mm (+17)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d8a94703bd9c57..25329e9e8a04ae 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -906,6 +906,8 @@ Bug Fixes to Compiler Builtins
 
 - Fix ``__builtin_source_location`` incorrectly returning wrong column for method chains. (#GH119129)
 
+- The behvaiour of ``__add_pointer`` and ``__remove_pointer`` for Objective-C++'s ``id`` and interfaces has been fixed.
+
 Bug Fixes to Attribute Support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 33d5378944ddbf..2781651b5d8f7d 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -1826,7 +1826,8 @@ QualType Sema::BuildPointerType(QualType T,
   if (checkQualifiedFunction(*this, T, Loc, QFK_Pointer))
     return QualType();
 
-  assert(!T->isObjCObjectType() && "Should build ObjCObjectPointerType");
+  if (T->isObjCObjectType())
+    return Context.getObjCObjectPointerType(T);
 
   // In ARC, it is forbidden to build pointers to unqualified pointers.
   if (getLangOpts().ObjCAutoRefCount)
@@ -9807,8 +9808,7 @@ QualType Sema::BuiltinAddPointer(QualType BaseType, SourceLocation Loc) {
 }
 
 QualType Sema::BuiltinRemovePointer(QualType BaseType, SourceLocation Loc) {
-  // We don't want block pointers or ObjectiveC's id type.
-  if (!BaseType->isAnyPointerType() || BaseType->isObjCIdType())
+  if (!BaseType->isAnyPointerType())
     return BaseType;
 
   return BaseType->getPointeeType();
diff --git a/clang/test/SemaCXX/remove_pointer.mm b/clang/test/SemaCXX/remove_pointer.mm
deleted file mode 100644
index d1cf1fa9f4efca..00000000000000
--- a/clang/test/SemaCXX/remove_pointer.mm
+++ /dev/null
@@ -1,8 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-
-// expected-no-diagnostics
-
-@class X;
-
-static_assert(__is_same(__remove_pointer(X *), X), "");
-static_assert(__is_same(__remove_pointer(id), id), "");
diff --git a/clang/test/SemaObjCXX/type-traits.mm b/clang/test/SemaObjCXX/type-traits.mm
new file mode 100644
index 00000000000000..81b9573b521929
--- /dev/null
+++ b/clang/test/SemaObjCXX/type-traits.mm
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=c++17  %s
+
+// expected-no-diagnostics
+
+@interface I;
+@end
+
+@class C;
+
+static_assert(__is_same(__add_pointer(id), id*));
+static_assert(__is_same(__add_pointer(I), I*));
+
+static_assert(__is_same(__remove_pointer(C*), C));
+static_assert(!__is_same(__remove_pointer(id), id));
+static_assert(__is_same(__remove_pointer(id*), id));
+static_assert(__is_same(__remove_pointer(__add_pointer(id)), id));
+static_assert(__is_same(__add_pointer(__remove_pointer(id)), id));

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This aligns the builtins with how implementations work which don't use
the buitins.
@tstellar tstellar merged commit 6195c3a into llvm:release/20.x Feb 11, 2025
8 of 11 checks passed
@github-actions
Copy link

@philnik777 (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

Development

Successfully merging this pull request may close these issues.

4 participants