[Sema] Fix parameter index checks on explicit object member functions #165586
Conversation
This file contains hidden or 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
With the C++23 explicit object parameter feature, it is no longer sufficient to only check if a function is an instance method to determine if it has an implicit this argument. That causes problems in attributes that have parameter indexes.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Vladimir Vuksanovic (vvuksanovic) ChangesWith the C++23 explicit object parameter feature, it is no longer sufficient to only check if a function is an instance method to determine if it has an implicit this argument. That causes problems in attributes that have parameter indexes. Full diff: https://github.com/llvm/llvm-project/pull/165586.diff 11 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index add1582344a0e..83318c76db820 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -451,6 +451,7 @@ Bug Fixes to Attribute Support
``[[gnu::error("some error")]]`` now correctly triggers an error. (#GH146520)
- Fix a crash when the function name is empty in the `swift_name` attribute. (#GH157075)
- Fixes crashes or missing diagnostics with the `device_kernel` attribute. (#GH161905)
+- Fix handling of parameter indexes when an attribute is applied to a C++23 explicit object member function.
Bug Fixes to C++ Support
^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/Attr.h b/clang/include/clang/AST/Attr.h
index ce273c167aa22..b28da60c11840 100644
--- a/clang/include/clang/AST/Attr.h
+++ b/clang/include/clang/AST/Attr.h
@@ -328,7 +328,8 @@ class ParamIdx {
: Idx(Idx), HasThis(false), IsValid(true) {
assert(Idx >= 1 && "Idx must be one-origin");
if (const auto *FD = dyn_cast<FunctionDecl>(D))
- HasThis = FD->isCXXInstanceMember();
+ HasThis = FD->isCXXInstanceMember() &&
+ !FD->hasCXXExplicitFunctionObjectParameter();
}
/// A type into which \c ParamIdx can be serialized.
diff --git a/clang/include/clang/Sema/Attr.h b/clang/include/clang/Sema/Attr.h
index 3f0b10212789a..ab977d148648f 100644
--- a/clang/include/clang/Sema/Attr.h
+++ b/clang/include/clang/Sema/Attr.h
@@ -123,6 +123,13 @@ inline bool isInstanceMethod(const Decl *D) {
return false;
}
+inline bool hasImplicitObjectParameter(const Decl *D) {
+ if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(D))
+ return MethodDecl->isInstance() &&
+ !MethodDecl->hasCXXExplicitFunctionObjectParameter();
+ return false;
+}
+
/// Diagnose mutually exclusive attributes when present on a given
/// declaration. Returns true if diagnosed.
template <typename AttrTy>
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 52904c72d1cfc..c67ed99b1f49e 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2608,13 +2608,13 @@ class Sema final : public SemaBase {
};
/// Given a function and its FormatAttr or FormatMatchesAttr info, attempts to
- /// populate the FomatStringInfo parameter with the attribute's correct
+ /// populate the FormatStringInfo parameter with the attribute's correct
/// format_idx and firstDataArg. Returns true when the format fits the
/// function and the FormatStringInfo has been populated.
static bool getFormatStringInfo(const Decl *Function, unsigned FormatIdx,
unsigned FirstArg, FormatStringInfo *FSI);
static bool getFormatStringInfo(unsigned FormatIdx, unsigned FirstArg,
- bool IsCXXMember, bool IsVariadic,
+ bool HasImplicitThisParam, bool IsVariadic,
FormatStringInfo *FSI);
// Used by C++ template instantiation.
@@ -5119,7 +5119,7 @@ class Sema final : public SemaBase {
// In C++ the implicit 'this' function parameter also counts.
// Parameters are counted from one.
bool HP = hasFunctionProto(D);
- bool HasImplicitThisParam = isInstanceMethod(D);
+ bool HasImplicitThisParam = hasImplicitObjectParameter(D);
bool IV = HP && isFunctionOrMethodVariadic(D);
unsigned NumParams =
(HP ? getFunctionOrMethodNumParams(D) : 0) + HasImplicitThisParam;
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index f4517877b04c8..ad2c2e4a97bb9 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3542,9 +3542,7 @@ bool Sema::ValueIsRunOfOnes(CallExpr *TheCall, unsigned ArgNum) {
bool Sema::getFormatStringInfo(const Decl *D, unsigned FormatIdx,
unsigned FirstArg, FormatStringInfo *FSI) {
- bool IsCXXMember = false;
- if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
- IsCXXMember = MD->isInstance();
+ bool HasImplicitThisParam = hasImplicitObjectParameter(D);
bool IsVariadic = false;
if (const FunctionType *FnTy = D->getFunctionType())
IsVariadic = cast<FunctionProtoType>(FnTy)->isVariadic();
@@ -3553,11 +3551,12 @@ bool Sema::getFormatStringInfo(const Decl *D, unsigned FormatIdx,
else if (const auto *OMD = dyn_cast<ObjCMethodDecl>(D))
IsVariadic = OMD->isVariadic();
- return getFormatStringInfo(FormatIdx, FirstArg, IsCXXMember, IsVariadic, FSI);
+ return getFormatStringInfo(FormatIdx, FirstArg, HasImplicitThisParam,
+ IsVariadic, FSI);
}
bool Sema::getFormatStringInfo(unsigned FormatIdx, unsigned FirstArg,
- bool IsCXXMember, bool IsVariadic,
+ bool HasImplicitThisParam, bool IsVariadic,
FormatStringInfo *FSI) {
if (FirstArg == 0)
FSI->ArgPassingKind = FAPK_VAList;
@@ -3571,7 +3570,7 @@ bool Sema::getFormatStringInfo(unsigned FormatIdx, unsigned FirstArg,
// The way the format attribute works in GCC, the implicit this argument
// of member functions is counted. However, it doesn't appear in our own
// lists, so decrement format_idx in that case.
- if (IsCXXMember) {
+ if (HasImplicitThisParam) {
if(FSI->FormatIdx == 0)
return false;
--FSI->FormatIdx;
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 964a2a791e18f..a9e7b44ac9d73 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3785,7 +3785,7 @@ static bool handleFormatAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL,
// In C++ the implicit 'this' function parameter also counts, and they are
// counted from one.
- bool HasImplicitThisParam = isInstanceMethod(D);
+ bool HasImplicitThisParam = hasImplicitObjectParameter(D);
Info->NumArgs = getFunctionOrMethodNumParams(D) + HasImplicitThisParam;
Info->Identifier = AL.getArgAsIdent(0)->getIdentifierInfo();
@@ -3926,7 +3926,7 @@ static void handleCallbackAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
return;
}
- bool HasImplicitThisParam = isInstanceMethod(D);
+ bool HasImplicitThisParam = hasImplicitObjectParameter(D);
int32_t NumArgs = getFunctionOrMethodNumParams(D);
FunctionDecl *FD = D->getAsFunction();
@@ -4110,7 +4110,7 @@ static void handleLifetimeCaptureByAttr(Sema &S, Decl *D,
}
void Sema::LazyProcessLifetimeCaptureByParams(FunctionDecl *FD) {
- bool HasImplicitThisParam = isInstanceMethod(FD);
+ bool HasImplicitThisParam = hasImplicitObjectParameter(FD);
SmallVector<LifetimeCaptureByAttr *, 1> Attrs;
for (ParmVarDecl *PVD : FD->parameters())
if (auto *A = PVD->getAttr<LifetimeCaptureByAttr>())
diff --git a/clang/test/SemaCXX/attr-callback-broken.cpp b/clang/test/SemaCXX/attr-callback-broken.cpp
index a5469b22ba350..53b331a49251b 100644
--- a/clang/test/SemaCXX/attr-callback-broken.cpp
+++ b/clang/test/SemaCXX/attr-callback-broken.cpp
@@ -1,7 +1,12 @@
-// RUN: %clang_cc1 %s -verify -fsyntax-only
+// RUN: %clang_cc1 %s -std=c++23 -verify -fsyntax-only
class C_in_class {
#define HAS_THIS
#include "../Sema/attr-callback-broken.c"
#undef HAS_THIS
};
+
+class ExplicitParameterObject {
+ __attribute__((callback(2, 0))) void explicit_this_idx(this ExplicitParameterObject* self, void (*callback)(ExplicitParameterObject*)); // expected-error {{'callback' argument at position 2 references unavailable implicit 'this'}}
+ __attribute__((callback(2, this))) void explicit_this_identifier(this ExplicitParameterObject* self, void (*callback)(ExplicitParameterObject*)); // expected-error {{'callback' argument at position 2 references unavailable implicit 'this'}}
+};
diff --git a/clang/test/SemaCXX/attr-callback.cpp b/clang/test/SemaCXX/attr-callback.cpp
index ee02f7d3d24f7..ff5a241e92f74 100644
--- a/clang/test/SemaCXX/attr-callback.cpp
+++ b/clang/test/SemaCXX/attr-callback.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -fsyntax-only
+// RUN: %clang_cc1 %s -std=c++23 -verify -fsyntax-only
// expected-no-diagnostics
@@ -6,6 +6,11 @@ class C_in_class {
#include "../Sema/attr-callback.c"
};
+class ExplicitParameterObject {
+ __attribute__((callback(2, 1))) void explicit_this_idx(this ExplicitParameterObject* self, void (*callback)(ExplicitParameterObject*));
+ __attribute__((callback(2, self))) void explicit_this_identifier(this ExplicitParameterObject* self, void (*callback)(ExplicitParameterObject*));
+};
+
struct Base {
void no_args_1(void (*callback)(void));
diff --git a/clang/test/SemaCXX/attr-format.cpp b/clang/test/SemaCXX/attr-format.cpp
index adc05fc46776c..bc2d840d50cbf 100644
--- a/clang/test/SemaCXX/attr-format.cpp
+++ b/clang/test/SemaCXX/attr-format.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wformat-nonliteral -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++23 -Wformat-nonliteral -verify %s
#include <stdarg.h>
int printf(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
@@ -11,6 +11,7 @@ struct S {
// the format argument is argument 2 here.
void g(const char*, ...) __attribute__((format(printf, 2, 3)));
const char* g2(const char*) __attribute__((format_arg(2)));
+ void g3(this S&, const char *, ...) __attribute__((format(printf, 2, 3)));
void h(const char*, ...) __attribute__((format(printf, 1, 4))); // \
expected-error{{implicit this argument as the format string}}
@@ -18,6 +19,8 @@ struct S {
expected-error{{out of bounds}}
const char* h3(const char*) __attribute__((format_arg(1))); // \
expected-error{{invalid for the implicit this argument}}
+ void h4(this S&, const char *, ...) __attribute__((format(printf, 1, 3))); // \
+ expected-error {{format argument not a string type}}
void operator() (const char*, ...) __attribute__((format(printf, 2, 3)));
};
diff --git a/clang/test/SemaCXX/attr-lifetime-capture-by.cpp b/clang/test/SemaCXX/attr-lifetime-capture-by.cpp
index 70a5fe5a45376..8606592c6b771 100644
--- a/clang/test/SemaCXX/attr-lifetime-capture-by.cpp
+++ b/clang/test/SemaCXX/attr-lifetime-capture-by.cpp
@@ -44,4 +44,7 @@ struct T {
{
s.captureInt(x);
}
+
+ void explicit_this1(this T& self, const int &x [[clang::lifetime_capture_by(self)]]);
+ void explicit_this2(this T& self, const int &x [[clang::lifetime_capture_by(this)]]); // expected-error {{argument references unavailable implicit 'this'}}
};
diff --git a/clang/test/SemaCXX/attr-nonnull.cpp b/clang/test/SemaCXX/attr-nonnull.cpp
index 6f9119b519d09..985c4213b4c80 100644
--- a/clang/test/SemaCXX/attr-nonnull.cpp
+++ b/clang/test/SemaCXX/attr-nonnull.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -std=c++23 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++23 -fsyntax-only -verify %s -fexperimental-new-constant-interpreter
struct S {
S(const char *) __attribute__((nonnull(2)));
@@ -11,6 +11,11 @@ struct S {
void h(const char*) __attribute__((nonnull(1))); // \
expected-error{{invalid for the implicit this argument}}
+
+ void i(this S* self, const char*) __attribute__((nonnull(1)));
+
+ void j(this S* self, const char*) __attribute__((nonnull(3))); // \
+ expected-error{{'nonnull' attribute parameter 1 is out of bounds}}
};
void test() {
|
Sirraide
reviewed
Oct 30, 2025
Sirraide
approved these changes
Oct 31, 2025
ckoparkar
added a commit
to ckoparkar/llvm-project
that referenced
this pull request
Oct 31, 2025
* main: [SPIRV] Fix vector bitcast check in LegalizePointerCast (llvm#164997) [lldb][docs] Add troubleshooting section to scripting introduction [Sema] Fix parameter index checks on explicit object member functions (llvm#165586) To fix polymorphic pointer assignment in FORALL when LHS is unlimited polymorphic and RHS is intrinsic type target (llvm#164999) [CostModel][AArch64] Model cost of extract.last.active intrinsic (clastb) (llvm#165739) [MemProf] Select largest of matching contexts from profile (llvm#165338) [lldb][TypeSystem] Better support for _BitInt types (llvm#165689) [NVPTX] Move TMA G2S lowering to Tablegen (llvm#165710) [MLIR][NVVM] Extend NVVM mma ops to support fp64 (llvm#165380) [UTC] Support to test annotated IR (llvm#165419)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
With the C++23 explicit object parameter feature, it is no longer sufficient to only check if a function is an instance method to determine if it has an implicit this argument. That causes problems in attributes that have parameter indexes.