Skip to content

Conversation

@dougsonos
Copy link
Contributor

@dougsonos dougsonos commented Jan 2, 2025

FunctionEffectsRef::get() is supposed to strip off layers of indirection (pointers/references, type sugar) to get to a FunctionProtoType (if any) and return its effects (if any).

It wasn't correctly dealing with situations where the compiler implicitly converts an array to a pointer.

@dougsonos dougsonos marked this pull request as ready for review January 3, 2025 00:59
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 3, 2025
@dougsonos dougsonos self-assigned this Jan 3, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 3, 2025

@llvm/pr-subscribers-clang

Author: Doug Wyatt (dougsonos)

Changes

FunctionEffectsRef::get() is supposed to strip off layers of indirection (pointers/references, type sugar) to get to a FunctionProtoType (if any) and return its effects (if any).

It wasn't correctly dealing with situations where the compiler implicitly converts an array to a pointer.


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

2 Files Affected:

  • (modified) clang/include/clang/AST/Type.h (+13-4)
  • (modified) clang/test/Sema/attr-nonblocking-constraints.cpp (+11)
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 09c98f642852fc..782c32f41852e2 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -8836,13 +8836,22 @@ void FixedPointValueToString(SmallVectorImpl<char> &Str, llvm::APSInt Val,
                              unsigned Scale);
 
 inline FunctionEffectsRef FunctionEffectsRef::get(QualType QT) {
+  const Type *TypePtr = QT.getTypePtr();
   while (true) {
-    QualType Pointee = QT->getPointeeType();
-    if (Pointee.isNull())
+    // Note that getPointeeType() seems to successfully navigate some constructs
+    // for which isAnyPointerType() returns false (e.g.
+    // pointer-to-member-function).
+    QualType Pointee = TypePtr->getPointeeType();
+    if (Pointee.isNull()) {
+      if (TypePtr->isArrayType()) {
+        TypePtr = TypePtr->getBaseElementTypeUnsafe();
+        continue;
+      }
       break;
-    QT = Pointee;
+    }
+    TypePtr = Pointee.getTypePtr();
   }
-  if (const auto *FPT = QT->getAs<FunctionProtoType>())
+  if (const auto *FPT = TypePtr->getAs<FunctionProtoType>())
     return FPT->getFunctionEffects();
   return {};
 }
diff --git a/clang/test/Sema/attr-nonblocking-constraints.cpp b/clang/test/Sema/attr-nonblocking-constraints.cpp
index bbc909f627f4c3..f7b5abbfa34e91 100644
--- a/clang/test/Sema/attr-nonblocking-constraints.cpp
+++ b/clang/test/Sema/attr-nonblocking-constraints.cpp
@@ -246,6 +246,17 @@ void PTMFTester::convert() [[clang::nonblocking]]
 	(this->*mConvertFunc)();
 }
 
+// Allow implicit conversion from array to pointer.
+void nb14(unsigned idx) [[clang::nonblocking]]
+{
+	using FP = void (*)() [[clang::nonblocking]];
+	using FPArray = FP[2];
+	auto nb = +[]() [[clang::nonblocking]] {};
+
+	FPArray src{ nb, nullptr };
+	FP f = src[idx]; // This should not generate a warning.
+}
+
 // Block variables
 void nb17(void (^blk)() [[clang::nonblocking]]) [[clang::nonblocking]] {
 	blk();

@dougsonos dougsonos requested a review from Sirraide January 3, 2025 01:01
@dougsonos
Copy link
Contributor Author

Ping

@github-actions
Copy link

github-actions bot commented Jan 16, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Add tests for multi-dimensional and variable-length arrays.
Copy link
Member

@Sirraide Sirraide left a comment

Choose a reason for hiding this comment

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

One nit but lgtm otherwise

@dougsonos dougsonos merged commit 0417cd1 into llvm:main Jan 17, 2025
8 checks passed
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

None yet

Development

Successfully merging this pull request may close these issues.

3 participants