[clang] Support [[]] spelling for noderef#190100
Open
Conversation
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang Author: PiJoules ChangesThis refactors the Changes include:
Note the bulk of this PR was made by gemini, but it has been thoroughly reviewed and edited by me to the best of my ability. Fixes: #55790 Patch is 29.32 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/190100.diff 10 Files Affected:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1b8a9803be472..9e17c5ba254eb 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8417,7 +8417,11 @@ class Sema final : public SemaBase {
/// keeping a container of all pending expressions and checking if the address
/// of them are eventually taken.
void CheckSubscriptAccessOfNoDeref(const ArraySubscriptExpr *E);
- void CheckAddressOfNoDeref(const Expr *E);
+
+ /// Check if the address of a `noderef` expression is taken. Return true if
+ /// the expression was marked with `noderef` and the attribute should be
+ /// propagated to the resulting pointer type.
+ bool CheckAddressOfNoDeref(const Expr *E);
///@}
diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index 0040f3aa1a891..d3f1259a3f15a 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -225,15 +225,11 @@ namespace {
void CheckNoDeref(Sema &S, const QualType FromType, const QualType ToType,
SourceLocation OpLoc) {
- if (const auto *PtrType = dyn_cast<PointerType>(FromType)) {
- if (PtrType->getPointeeType()->hasAttr(attr::NoDeref)) {
- if (const auto *DestType = dyn_cast<PointerType>(ToType)) {
- if (!DestType->getPointeeType()->hasAttr(attr::NoDeref)) {
- S.Diag(OpLoc, diag::warn_noderef_to_dereferenceable_pointer);
- }
- }
- }
- }
+ // Note `getAs` strips the sugar to ensure it's a real pointer, but we still
+ // use the original type since the attribute is part of the sugar.
+ if (FromType->getAs<PointerType>() && FromType->hasAttr(attr::NoDeref) &&
+ ToType->getAs<PointerType>() && !ToType->hasAttr(attr::NoDeref))
+ S.Diag(OpLoc, diag::warn_noderef_to_dereferenceable_pointer);
}
struct CheckNoDerefRAII {
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 63a70e4b77bc4..30111b912dd36 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7445,13 +7445,6 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
// processTypeAttr().
break;
}
-
- if (AL.getKind() == ParsedAttr::AT_NoDeref) {
- // FIXME: `noderef` currently doesn't work correctly in [[]] syntax.
- // See https://github.com/llvm/llvm-project/issues/55790 for details.
- // We allow processTypeAttrs() to emit a warning and silently move on.
- break;
- }
}
// N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
// statement attribute is not written on a declaration, but this code is
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index c9642ed298bf3..2758103fe9af5 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -549,8 +549,14 @@ ExprResult Sema::DefaultFunctionArrayConversion(Expr *E, bool Diagnose) {
// T" can be converted to an rvalue of type "pointer to T".
//
if (getLangOpts().C99 || getLangOpts().CPlusPlus || E->isLValue()) {
- ExprResult Res = ImpCastExprToType(E, Context.getArrayDecayedType(Ty),
- CK_ArrayToPointerDecay);
+ QualType PtrTy = Context.getArrayDecayedType(Ty);
+
+ // When the array is `noderef` and we promote to a pointer, also apply
+ // `noderef` on the pointer.
+ if (Ty->hasAttr(attr::NoDeref))
+ PtrTy = Context.getAttributedType(attr::NoDeref, PtrTy, PtrTy);
+
+ ExprResult Res = ImpCastExprToType(E, PtrTy, CK_ArrayToPointerDecay);
if (Res.isInvalid())
return ExprError();
E = Res.get();
@@ -5327,7 +5333,7 @@ ExprResult Sema::CreateBuiltinMatrixSubscriptExpr(Expr *Base, Expr *RowIdx,
MTy->getElementType(), RBLoc);
}
-void Sema::CheckAddressOfNoDeref(const Expr *E) {
+bool Sema::CheckAddressOfNoDeref(const Expr *E) {
ExpressionEvaluationContextRecord &LastRecord = ExprEvalContexts.back();
const Expr *StrippedExpr = E->IgnoreParenImpCasts();
@@ -5337,7 +5343,7 @@ void Sema::CheckAddressOfNoDeref(const Expr *E) {
while ((Member = dyn_cast<MemberExpr>(StrippedExpr)) && !Member->isArrow())
StrippedExpr = Member->getBase()->IgnoreParenImpCasts();
- LastRecord.PossibleDerefs.erase(StrippedExpr);
+ return LastRecord.PossibleDerefs.erase(StrippedExpr);
}
void Sema::CheckSubscriptAccessOfNoDeref(const ArraySubscriptExpr *E) {
@@ -5351,15 +5357,16 @@ void Sema::CheckSubscriptAccessOfNoDeref(const ArraySubscriptExpr *E) {
if (isa<ArrayType>(ResultTy))
return;
- if (ResultTy->hasAttr(attr::NoDeref)) {
+ const Expr *Base = E->getBase();
+ QualType BaseTy = Base->getType();
+
+ if (BaseTy->hasAttr(attr::NoDeref)) {
LastRecord.PossibleDerefs.insert(E);
return;
}
// Check if the base type is a pointer to a member access of a struct
// marked with noderef.
- const Expr *Base = E->getBase();
- QualType BaseTy = Base->getType();
if (!(isa<ArrayType>(BaseTy) || isa<PointerType>(BaseTy)))
// Not a pointer access
return;
@@ -5369,8 +5376,8 @@ void Sema::CheckSubscriptAccessOfNoDeref(const ArraySubscriptExpr *E) {
Member->isArrow())
Base = Member->getBase();
- if (const auto *Ptr = dyn_cast<PointerType>(Base->getType())) {
- if (Ptr->getPointeeType()->hasAttr(attr::NoDeref))
+ if (Base->getType()->getAs<PointerType>()) {
+ if (Base->getType()->hasAttr(attr::NoDeref))
LastRecord.PossibleDerefs.insert(E);
}
}
@@ -10127,10 +10134,10 @@ AssignConvertType Sema::CheckSingleAssignmentConstraints(QualType LHSType,
ExprResult LocalRHS = CallerRHS;
ExprResult &RHS = ConvertRHS ? CallerRHS : LocalRHS;
- if (const auto *LHSPtrType = LHSType->getAs<PointerType>()) {
- if (const auto *RHSPtrType = RHS.get()->getType()->getAs<PointerType>()) {
- if (RHSPtrType->getPointeeType()->hasAttr(attr::NoDeref) &&
- !LHSPtrType->getPointeeType()->hasAttr(attr::NoDeref)) {
+ if (LHSType->getAs<PointerType>()) {
+ if (RHS.get()->getType()->getAs<PointerType>()) {
+ if (RHS.get()->getType()->hasAttr(attr::NoDeref) &&
+ !LHSType->hasAttr(attr::NoDeref)) {
Diag(RHS.get()->getExprLoc(),
diag::warn_noderef_to_dereferenceable_pointer)
<< RHS.get()->getSourceRange();
@@ -16203,7 +16210,13 @@ ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc,
break;
case UO_AddrOf:
resultType = CheckAddressOfOperand(Input, OpLoc);
- CheckAddressOfNoDeref(InputExpr);
+
+ // Since the expression is noderef, the resulting type should also be
+ // noderef.
+ if (CheckAddressOfNoDeref(InputExpr))
+ resultType =
+ Context.getAttributedType(attr::NoDeref, resultType, resultType);
+
RecordModifiableNonNullParam(*this, InputExpr);
break;
case UO_Deref: {
@@ -16404,7 +16417,7 @@ ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc,
UnaryOperator::Create(Context, Input.get(), Opc, resultType, VK, OK,
OpLoc, CanOverflow, CurFPFeatureOverrides());
- if (Opc == UO_Deref && UO->getType()->hasAttr(attr::NoDeref) &&
+ if (Opc == UO_Deref && Input.get()->getType()->hasAttr(attr::NoDeref) &&
!isa<ArrayType>(UO->getType().getDesugaredType(Context)) &&
!isUnevaluatedContext())
ExprEvalContexts.back().PossibleDerefs.insert(UO);
@@ -18199,17 +18212,11 @@ const DeclRefExpr *CheckPossibleDeref(Sema &S, const Expr *PossibleDeref) {
} else if (const auto *E = dyn_cast<MemberExpr>(PossibleDeref)) {
return CheckPossibleDeref(S, E->getBase());
} else if (const auto E = dyn_cast<DeclRefExpr>(PossibleDeref)) {
- QualType Inner;
QualType Ty = E->getType();
- if (const auto *Ptr = Ty->getAs<PointerType>())
- Inner = Ptr->getPointeeType();
- else if (const auto *Arr = S.Context.getAsArrayType(Ty))
- Inner = Arr->getElementType();
- else
- return nullptr;
-
- if (Inner->hasAttr(attr::NoDeref))
- return E;
+ if (Ty->getAs<PointerType>() || S.Context.getAsArrayType(Ty)) {
+ if (Ty->hasAttr(attr::NoDeref))
+ return E;
+ }
}
return nullptr;
}
diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index ff7c844f66123..f06aaad0663fb 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -1766,9 +1766,8 @@ void Sema::CheckMemberAccessOfNoDeref(const MemberExpr *E) {
CheckAddressOfNoDeref(E->getBase());
}
} else if (E->isArrow()) {
- if (const auto *Ptr = dyn_cast<PointerType>(
- E->getBase()->getType().getDesugaredType(Context))) {
- if (Ptr->getPointeeType()->hasAttr(attr::NoDeref))
+ if (E->getBase()->getType()->getAs<PointerType>()) {
+ if (E->getBase()->getType()->hasAttr(attr::NoDeref))
ExprEvalContexts.back().PossibleDerefs.insert(E);
}
}
@@ -1831,7 +1830,7 @@ Sema::BuildFieldReferenceExpr(Expr *BaseExpr, bool IsArrow,
// result. E.g. the expression
// &someNoDerefPtr->pointerMember
// should be a noderef pointer again.
- if (BaseType->hasAttr(attr::NoDeref))
+ if (BaseExpr->getType()->hasAttr(attr::NoDeref))
MemberType =
Context.getAttributedType(attr::NoDeref, MemberType, MemberType);
}
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 6c929de9418ed..acbc547060fef 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -8332,11 +8332,10 @@ ExprResult InitializationSequence::Perform(Sema &S,
case SK_ConversionSequence:
case SK_ConversionSequenceNoNarrowing: {
- if (const auto *FromPtrType =
- CurInit.get()->getType()->getAs<PointerType>()) {
- if (const auto *ToPtrType = Step->Type->getAs<PointerType>()) {
- if (FromPtrType->getPointeeType()->hasAttr(attr::NoDeref) &&
- !ToPtrType->getPointeeType()->hasAttr(attr::NoDeref)) {
+ if (CurInit.get()->getType()->getAs<PointerType>()) {
+ if (Step->Type->getAs<PointerType>()) {
+ if (CurInit.get()->getType()->hasAttr(attr::NoDeref) &&
+ !Step->Type->hasAttr(attr::NoDeref)) {
// Do not check static casts here because they are checked earlier
// in Sema::ActOnCXXNamedCast()
if (!Kind.isStaticCast()) {
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 846474fe94adf..799fc100eb27d 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -47,6 +47,7 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/IR/DerivedTypes.h"
#include "llvm/Support/ErrorHandling.h"
+#include <algorithm>
#include <bitset>
#include <optional>
@@ -217,10 +218,6 @@ namespace {
/// stored in a MacroQualifiedTypeLoc.
llvm::DenseMap<const MacroQualifiedType *, SourceLocation> LocsForMacros;
- /// Flag to indicate we parsed a noderef attribute. This is used for
- /// validating that noderef was used on a pointer or array.
- bool parsedNoDeref;
-
// Flag to indicate that we already parsed a HLSL parameter modifier
// attribute. This prevents double-mutating the type.
bool ParsedHLSLParamMod;
@@ -228,7 +225,7 @@ namespace {
public:
TypeProcessingState(Sema &sema, Declarator &declarator)
: sema(sema), declarator(declarator),
- chunkIndex(declarator.getNumTypeObjects()), parsedNoDeref(false),
+ chunkIndex(declarator.getNumTypeObjects()),
ParsedHLSLParamMod(false) {}
Sema &getSema() const {
@@ -360,10 +357,6 @@ namespace {
LocsForMacros[MQT] = Loc;
}
- void setParsedNoDeref(bool parsed) { parsedNoDeref = parsed; }
-
- bool didParseNoDeref() const { return parsedNoDeref; }
-
void setParsedHLSLParamMod(bool Parsed) { ParsedHLSLParamMod = Parsed; }
bool didParseHLSLParamMod() const { return ParsedHLSLParamMod; }
@@ -4688,8 +4681,20 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
}
}
+ // GNU attributes on a declaration (e.g., __attribute__((noderef)) int *p;)
+ // are parsed as part of the DeclSpec, but they are intended to apply to
+ // the first pointer or array declarator chunk encountered. Standard
+ // C++11 attribute syntax `[[clang::noderef]]` has stricter placement
+ // rules and should not "float" to the pointer. If a standard noderef
+ // attribute is found on the DeclSpec, we skip setting `ExpectNoDerefChunk`
+ // to ensure it is diagnosed for being applied to a non-pointer type,
+ // rather than silently moving it to the pointer.
bool ExpectNoDerefChunk =
- state.getCurrentAttributes().hasAttribute(ParsedAttr::AT_NoDeref);
+ std::any_of(state.getCurrentAttributes().begin(),
+ state.getCurrentAttributes().end(), [](const ParsedAttr &AL) {
+ return AL.getKind() == ParsedAttr::AT_NoDeref &&
+ !AL.isStandardAttributeSyntax();
+ });
// Walk the DeclTypeInfo, building the recursive type as we go.
// DeclTypeInfos are ordered from the identifier out, which is
@@ -5471,17 +5476,58 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
processTypeAttrs(state, T, TAL_DeclChunk, DeclType.getAttrs(),
S.CUDA().IdentifyTarget(D.getAttributes()));
- if (DeclType.Kind != DeclaratorChunk::Paren) {
- if (ExpectNoDerefChunk && !IsNoDerefableChunk(DeclType))
+ if (DeclType.Kind != DeclaratorChunk::Paren && ExpectNoDerefChunk) {
+ // This block handles explicit indirection in the declaration
+ //
+ // __attribute__((noderef)) int *p;
+ //
+ // If we are looking for a GNU-style noderef attribute that was declared
+ // on the DeclSpec, we try to apply it to the first pointer or array
+ // declarator chunk encountered. Once we apply it (or diagnose a warning),
+ // we set ExpectNoDerefChunk to false so that it only applies to the
+ // outermost pointer or array chunk and doesn't "bleed" into deeper
+ // indirection levels as this loop continues.
+ if (!IsNoDerefableChunk(DeclType)) {
S.Diag(DeclType.Loc, diag::warn_noderef_on_non_pointer_or_array);
-
- ExpectNoDerefChunk = state.didParseNoDeref();
+ } else {
+ for (ParsedAttr &AL :
+ state.getDeclarator().getMutableDeclSpec().getAttributes()) {
+ if (AL.getKind() == ParsedAttr::AT_NoDeref) {
+ T = state.getAttributedType(
+ createSimpleAttr<NoDerefAttr>(S.Context, AL), T, T);
+ break;
+ }
+ }
+ }
+ ExpectNoDerefChunk = false;
}
}
- if (ExpectNoDerefChunk)
- S.Diag(state.getDeclarator().getBeginLoc(),
- diag::warn_noderef_on_non_pointer_or_array);
+ // This block handles implicit indirection, such as through a typedef or when
+ // the base type is already a pointer
+ //
+ // typedef int *IntPtr;
+ // __attribute__((noderef)) IntPtr p;
+ //
+ // If the loop above finished without finding an explicit pointer or array
+ // chunk to attach the attribute to, we check the final resolved type and
+ // apply it if it's a pointer or array.
+ if (ExpectNoDerefChunk) {
+ if (T->isPointerType() || T->isArrayType()) {
+ for (ParsedAttr &AL :
+ state.getDeclarator().getMutableDeclSpec().getAttributes()) {
+ if (AL.getKind() == ParsedAttr::AT_NoDeref) {
+ ASTContext &Ctx = S.Context;
+ T = state.getAttributedType(createSimpleAttr<NoDerefAttr>(Ctx, AL), T,
+ T);
+ break;
+ }
+ }
+ } else {
+ S.Diag(state.getDeclarator().getBeginLoc(),
+ diag::warn_noderef_on_non_pointer_or_array);
+ }
+ }
// GNU warning -Wstrict-prototypes
// Warn if a function declaration or definition is without a prototype.
@@ -9008,7 +9054,6 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
const ParsedAttributesView &attrs,
CUDAFunctionTarget CFT) {
- state.setParsedNoDeref(false);
if (attrs.empty())
return;
@@ -9175,20 +9220,16 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
break;
case ParsedAttr::AT_NoDeref: {
- // FIXME: `noderef` currently doesn't work correctly in [[]] syntax.
- // See https://github.com/llvm/llvm-project/issues/55790 for details.
- // For the time being, we simply emit a warning that the attribute is
- // ignored.
- if (attr.isStandardAttributeSyntax()) {
- state.getSema().Diag(attr.getLoc(), diag::warn_attribute_ignored)
- << attr;
- break;
- }
- ASTContext &Ctx = state.getSema().Context;
- type = state.getAttributedType(createSimpleAttr<NoDerefAttr>(Ctx, attr),
- type, type);
attr.setUsedAsTypeAttr();
- state.setParsedNoDeref(true);
+ if (!state.isProcessingDeclSpec() || attr.isStandardAttributeSyntax()) {
+ if (!type->isPointerType() && !type->isArrayType()) {
+ state.getSema().Diag(attr.getLoc(),
+ diag::warn_noderef_on_non_pointer_or_array);
+ }
+ ASTContext &Ctx = state.getSema().Context;
+ type = state.getAttributedType(createSimpleAttr<NoDerefAttr>(Ctx, attr),
+ type, type);
+ }
break;
}
diff --git a/clang/test/Frontend/noderef.c b/clang/test/Frontend/noderef.c
index 3f0d8269ca88d..dbbc39e66d75b 100644
--- a/clang/test/Frontend/noderef.c
+++ b/clang/test/Frontend/noderef.c
@@ -31,11 +31,11 @@ int test(void) {
x = *((int NODEREF *)p2); // expected-warning{{dereferencing expression marked as 'noderef'}}
int NODEREF **q;
- int *NODEREF *q2; // expected-note 4 {{q2 declared here}}
+ int *NODEREF *q2;
// Indirection
x = **q; // expected-warning{{dereferencing expression marked as 'noderef'}}
- p2 = *q2; // expected-warning{{dereferencing q2; was declared with a 'noderef' type}}
+ p2 = *q2; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
**q; // expected-warning{{dereferencing expression marked as 'noderef'}}
@@ -51,7 +51,7 @@ int test(void) {
*p = 2; // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
*q = p; // ok
**q = 2; // expected-warning{{dereferencing expression marked as 'noderef'}}
- *q2 = p2; // expected-warning{{dereferencing q2; was declared with a 'noderef' type}}
+ *q2 = p2; // ok - casting from normal int pointer to a NODEREF int pointer
p = p + 1;
p = &*(p + 1);
@@ -99,7 +99,7 @@ int test(void) {
// Subscript access
x = p[1]; // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
x = q[0][0]; // expected-warning{{dereferencing expression marked as 'noderef'}}
- p2 = q2[0]; // expected-warning{{dereferencing q2; was declared with a 'noderef' type}}
+ p2 = q2[0]; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
p = q[*p]; // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
x = p[*p]; // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
// expected-warning@-1{{dereferencing p; was declared with a 'noderef' type}}
@@ -119,7 +119,11 @@ int test(void) {
p = s2_arr[1]->a;
p = s2_arr[1]->b; // expected-warning{{dereferencing expression marked as 'noderef'}}
- int *NODEREF *bptr = &s2_arr[1]->b;
+
+ // `&s2_arr[1]->b` is a NODEREF pointer to a normal int pointer.
+ // `bptr` is a normal pointer to a NODEREF int pointer.
+ // Assigning the former to the latter strips NODEREF from the outer pointer.
+ int *NODEREF *bptr = &s2_arr[1]->b; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
x = s2->s2->a; // expected-warning{{dereferencing expression marked as 'noderef'}}
x = s2_noderef->a[1]; // expected-warning{{dereferencing s2_noderef; was declared with a 'noderef' type}}
@@ -155,18 +159,25 @@ int test(void) {
typedef int_t NODEREF *(noderef_int_t);
typedef noderef_int_t *noderef_int_nest...
[truncated]
|
zwuis
reviewed
Apr 2, 2026
This refactors the `noderef` attribute implementation to correctly associate the attribute with pointer and array types rather than the pointee type, fulfilling the requirements for the bracket attribute syntax `[[clang::noderef]]`. Changes include: - Remove the `parsedNoDeref` logic which would apply noderef to the right (pointee) type rather than the left (pointer) type. - Use `getAs<Type>` instead of `dyn_cast<Type>` where appropriate to strip type sugar. - Fixes for some of the tests where we incorrectly associate a noderef with a pointee type rather than a pointer. - Add regression tests Fixes: llvm#55790
Contributor
Author
|
ping |
erichkeane
reviewed
Apr 7, 2026
Collaborator
erichkeane
left a comment
erichkeane
left a comment
There was a problem hiding this comment.
This attribute has been around almost 10 years now (ad7ac96), so changing the behavior this much seems inappropriate. I don't mind adding the spelling change, but doing this much work UNDER The context of changing the spelling is inappropriate.
shafik
reviewed
Apr 7, 2026
Collaborator
shafik
left a comment
shafik
left a comment
There was a problem hiding this comment.
I agree w/ Erich this seems like a pretty big behavior change ten years later. It is unclear to me reading the bug report this would be anything more than a convenience over actually enabling behavior that can't be achieved otherwise.
Also do we have any idea how much code this would break in the wild?
AaronBallman
reviewed
Apr 7, 2026
Contributor
AaronBallman
left a comment
AaronBallman
left a comment
There was a problem hiding this comment.
Thank you for working on this! I don't agree with everything in the issue, but these are bugs in Clang IMO:
// Actual: warning: 'noderef' can only be used on an array or pointer type
// Desired: Allow attribute in this position, diagnose the faulty dereference
int * [[clang::noderef]] p4;
*p4;
// Actual: error: 'noderef' attribute cannot be applied to types
// Desired: Either diagnose the faulty dereference or forbid `noderef` on typedefs
typedef int *IntPtr;
IntPtr [[clang::noderef]] p6;
*p6;
for the C++ spelling itself. However, for the __attribute__ spelling, it's less clear that it's a bug as opposed to the way things were originally designed and we have to live with that decision.
| } | ||
|
|
||
| namespace GH55790 { | ||
| [[clang::noderef]] int i; // expected-error{{'clang::noderef' attribute cannot be applied to a declaration}} expected-warning{{'noderef' can only be used on an array or pointer type}} |
Contributor
There was a problem hiding this comment.
We should not be getting two diagnostics for the same thing, but also, this is an "attribute ignored" situation generally, why is it issued as an error for noderef? e.g., https://godbolt.org/z/K4oTos3n3
| [[clang::noderef]] int i; // expected-error{{'clang::noderef' attribute cannot be applied to a declaration}} expected-warning{{'noderef' can only be used on an array or pointer type}} | ||
|
|
||
| [[clang::noderef]] int *p1; // expected-error{{'clang::noderef' attribute cannot be applied to a declaration}} expected-note{{p1 declared here}} | ||
| int i1 = *p1; // expected-warning{{dereferencing p1; was declared with a 'noderef' type}} |
Contributor
There was a problem hiding this comment.
I think we should be dropping the attribute so we don't get follow-on diagnostics.
| // Indirection | ||
| x = **q; // expected-warning{{dereferencing expression marked as 'noderef'}} | ||
| p2 = *q2; // expected-warning{{dereferencing q2; was declared with a 'noderef' type}} | ||
| p2 = *q2; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}} |
Contributor
There was a problem hiding this comment.
As others mentioned, I'm worried about changing the behavior here. __attribute__ spellings do slide around to appertain to whatever makes the most sense, unlike [[]] spellings, so we could elect to leave the behavior the same in this case due to the spelling used. (Whether we should or not is another thing.)
Given the number of uses in the wild (https://sourcegraph.com/search?q=context:global+__attribute__%28%28noderef%29%29+-file:.*test.*&patternType=keyword&sm=0) I am nervous.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 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.
This refactors the
noderefattribute implementation to correctly associate the attribute with pointer and array types rather than the pointee type, fulfilling the requirements for the bracket attribute syntax[[clang::noderef]].Changes include:
parsedNoDereflogic which would apply noderef to the right (pointee) type rather than the left (pointer) type.getAs<Type>instead ofdyn_cast<Type>where appropriate to strip type sugar.Note the bulk of this PR was made by gemini, but it has been thoroughly reviewed and edited by me to the best of my ability.
Fixes: #55790