[clang] Support member function poiners in Decl::getFunctionType() #125077
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This seems consistent with the documentation which claims it: ``` /// Looks through the Decl's underlying type to extract a FunctionType /// when possible. Will return null if the type underlying the Decl does not /// have a FunctionType. const FunctionType *getFunctionType(bool BlocksToo = true) const; ``` Without this, attaching attributes (which use `HasFunctionProto`) to member function pointers errors with: ``` error: '<attr>' only applies to non-K&R-style functions ``` ...which does not really make sense, since member functions are not K&C functions. With this change the Arm SME TypeAttrs work correctly on member function pointers. Note, however, that not all attributes work correctly when applied to function pointers or member function pointers. For example, `alloc_align` crashes when applied to a function pointer (on truck): https://godbolt.org/z/YvMhnhKfx (as it only expects a `FunctionDecl` not a `ParmVarDecl`). The same crash applies to member function pointers (for the same reason).
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Benjamin Maxwell (MacDue) ChangesThis seems consistent with the documentation which claims it: Without this, attaching attributes (which use ...which does not really make sense, since member functions are not K&C functions. With this change the Arm SME TypeAttrs work correctly on member function pointers. Note, however, that not all attributes work correctly when applied to function pointers or member function pointers. For example, Full diff: https://github.com/llvm/llvm-project/pull/125077.diff 5 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index f4ba2bc3c6de31..2a3a29bd2ee1cf 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -198,7 +198,7 @@ def OpenCLKernelFunction
// inclusive nature of subject testing).
def HasFunctionProto : SubsetSubject<DeclBase,
[{(S->getFunctionType(true) != nullptr &&
- isa<FunctionProtoType>(S->getFunctionType())) ||
+ isa<FunctionProtoType>(S->getFunctionType())) ||
isa<ObjCMethodDecl>(S) ||
isa<BlockDecl>(S)}],
"non-K&R-style functions">;
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index c0a331d18cab8d..fc16448cf9e905 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1203,6 +1203,8 @@ const FunctionType *Decl::getFunctionType(bool BlocksToo) const {
if (Ty->isFunctionPointerType())
Ty = Ty->castAs<PointerType>()->getPointeeType();
+ else if (Ty->isMemberFunctionPointerType())
+ Ty = Ty->castAs<MemberPointerType>()->getPointeeType();
else if (Ty->isFunctionReferenceType())
Ty = Ty->castAs<ReferenceType>()->getPointeeType();
else if (BlocksToo && Ty->isBlockPointerType())
diff --git a/clang/test/AST/attr-print-emit.cpp b/clang/test/AST/attr-print-emit.cpp
index a9bca6778d0f1a..77826f8f9af098 100644
--- a/clang/test/AST/attr-print-emit.cpp
+++ b/clang/test/AST/attr-print-emit.cpp
@@ -91,3 +91,8 @@ ANNOTATE_ATTR NONNULL_ATTR void fn_non_null_annotated_attr(int *) __attribute__(
[[gnu::nonnull(1)]] [[gnu::always_inline]] void cxx11_attr(int*) ANNOTATE_ATTR;
// CHECK: {{\[\[}}gnu::nonnull(1)]] {{\[\[}}gnu::always_inline]] void cxx11_attr(int *) __attribute__((annotate("Annotated")));
+
+struct Foo;
+
+// CHECK: void as_member_fn_ptr(int *(Foo::*member)(int) __attribute__((alloc_size(1))));
+void as_member_fn_ptr(int* (Foo::*member)(int) __attribute__((alloc_size(1))));
diff --git a/clang/test/CodeGen/AArch64/sme-attributes-member-function-pointer.cpp b/clang/test/CodeGen/AArch64/sme-attributes-member-function-pointer.cpp
new file mode 100644
index 00000000000000..ee784c816a0606
--- /dev/null
+++ b/clang/test/CodeGen/AArch64/sme-attributes-member-function-pointer.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -triple aarch64 -target-feature +sme -target-feature +sme2 -x c++ -std=c++20 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK
+
+struct TestStruct;
+
+__arm_new("za", "zt0") void test(TestStruct& TS,
+ void (TestStruct::*streaming_member_ptr)() __arm_streaming,
+ void (TestStruct::*streaming_compat_member)() __arm_streaming_compatible,
+ void (TestStruct::*arm_in_member)() __arm_in("za", "zt0"),
+ void (TestStruct::*arm_inout_member)() __arm_inout("za", "zt0"),
+ void (TestStruct::*arm_preserves_member)() __arm_preserves("za", "zt0"),
+ void (TestStruct::*arm_agnostic_member)() __arm_agnostic("sme_za_state")) {
+
+ // CHECK: call void %{{.*}} [[STREAMING_MEMBER_CALL_ATTRS:#.+]]
+ (TS.*streaming_member_ptr)();
+
+ // CHECK: call void %{{.*}} [[STREAMING_COMPAT_MEMBER_CALL_ATTRS:#.+]]
+ (TS.*streaming_compat_member)();
+
+ // CHECK: call void %{{.*}} [[ARM_IN_MEMBER_CALL_ATTRS:#.+]]
+ (TS.*arm_in_member)();
+
+ // CHECK: call void %{{.*}} [[ARM_INOUT_MEMBER_CALL_ATTRS:#.+]]
+ (TS.*arm_inout_member)();
+
+ // CHECK: call void %{{.*}} [[ARM_PRESERVES_MEMBER_CALL_ATTRS:#.+]]
+ (TS.*arm_preserves_member)();
+
+ // CHECK: call void %{{.*}} [[ARM_AGNOSTIC_MEMBER_CALL_ATTRS:#.+]]
+ (TS.*arm_agnostic_member)();
+}
+
+// CHECK: attributes [[STREAMING_MEMBER_CALL_ATTRS]] = { "aarch64_pstate_sm_enabled" }
+// CHECK: attributes [[STREAMING_COMPAT_MEMBER_CALL_ATTRS]] = { "aarch64_pstate_sm_compatible" }
+// CHECK: attributes [[ARM_IN_MEMBER_CALL_ATTRS]] = { "aarch64_in_za" "aarch64_in_zt0" }
+// CHECK: attributes [[ARM_INOUT_MEMBER_CALL_ATTRS]] = { "aarch64_inout_za" "aarch64_inout_zt0" }
+// CHECK: attributes [[ARM_PRESERVES_MEMBER_CALL_ATTRS]] = { "aarch64_preserves_za" "aarch64_preserves_zt0" }
+// CHECK: attributes [[ARM_AGNOSTIC_MEMBER_CALL_ATTRS]] = { "aarch64_za_state_agnostic" }
diff --git a/clang/test/CodeGen/xfail-alloc-align-fn-pointers.cpp b/clang/test/CodeGen/xfail-alloc-align-fn-pointers.cpp
new file mode 100644
index 00000000000000..80067500284b1e
--- /dev/null
+++ b/clang/test/CodeGen/xfail-alloc-align-fn-pointers.cpp
@@ -0,0 +1,10 @@
+
+// RUN: %clang_cc1 %s
+
+// FIXME: These should not crash!
+// XFAIL: *
+
+void aa_fn_ptr(char* (*member)(char*) __attribute__((alloc_align(1))));
+
+struct Test;
+void aa_member_fn_ptr(char* (Test::*member)(char*) __attribute__((alloc_align(1))));
|
|
Created an issue for the |
efriedma-quic
approved these changes
Jan 30, 2025
There was a problem hiding this comment.
LGTM... but I have a couple comments on things you mentioned in the commit message.
/// Looks through the Decl's underlying type to extract a FunctionType /// when possible. Will return null if the type underlying the Decl does not /// have a FunctionType.
Maybe the description here should more explicitly state that it looks for function pointers as well as functions...
error: '' only applies to non-K&R-style functions
Probably we should fix the error message not to mention K&R unless we actually find a K&R-style function type.
Good idea 👍 I've attempted to clarify what this function does now (in the doc comment).
I agree, but I think that can be done it a later patch. |
efriedma-quic
approved these changes
Jan 31, 2025
|
/cherry-pick 692c9b2 |
|
/pull-request #125956 |
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Feb 6, 2025
…lvm#125077) This seems consistent with the documentation, which claims it: ``` /// Looks through the Decl's underlying type to extract a FunctionType /// when possible. Will return null if the type underlying the Decl does not /// have a FunctionType. const FunctionType *getFunctionType(bool BlocksToo = true) const; ``` Note: This patch rewords this doc comment to clarify it includes various function pointer types. Without this, attaching attributes (which use `HasFunctionProto`) to member function pointers errors with: ``` error: '<attr>' only applies to non-K&R-style functions ``` ...which does not really make sense, since member functions are not K&C functions. With this change the Arm SME TypeAttrs work correctly on member function pointers. Note, however, that not all attributes work correctly when applied to function pointers or member function pointers. For example, `alloc_align` crashes when applied to a function pointer (on truck): https://godbolt.org/z/YvMhnhKfx (as it only expects a `FunctionDecl` not a `ParmVarDecl`). The same crash applies to member function pointers (for the same reason). (cherry picked from commit 692c9b2)
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
…lvm#125077) This seems consistent with the documentation, which claims it: ``` /// Looks through the Decl's underlying type to extract a FunctionType /// when possible. Will return null if the type underlying the Decl does not /// have a FunctionType. const FunctionType *getFunctionType(bool BlocksToo = true) const; ``` Note: This patch rewords this doc comment to clarify it includes various function pointer types. Without this, attaching attributes (which use `HasFunctionProto`) to member function pointers errors with: ``` error: '<attr>' only applies to non-K&R-style functions ``` ...which does not really make sense, since member functions are not K&C functions. With this change the Arm SME TypeAttrs work correctly on member function pointers. Note, however, that not all attributes work correctly when applied to function pointers or member function pointers. For example, `alloc_align` crashes when applied to a function pointer (on truck): https://godbolt.org/z/YvMhnhKfx (as it only expects a `FunctionDecl` not a `ParmVarDecl`). The same crash applies to member function pointers (for the same reason).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This seems consistent with the documentation, which claims it:
Note: This patch rewords this doc comment to clarify it includes various function pointer types.
Without this, attaching attributes (which use
HasFunctionProto) to member function pointers errors with:...which does not really make sense, since member functions are not K&C functions.
With this change the Arm SME TypeAttrs work correctly on member function pointers.
Note, however, that not all attributes work correctly when applied to function pointers or member function pointers. For example,
alloc_aligncrashes when applied to a function pointer (on truck): https://godbolt.org/z/YvMhnhKfx (as it only expects aFunctionDeclnot aParmVarDecl). The same crash applies to member function pointers (for the same reason).