Skip to content

Commit 77baccc

Browse files
committed
[clang][nullability] allow _Nonnull etc on gsl::Pointer types
This enables external nullability checkers to make use of these annotations on smart pointer types. Existing static warnings for raw pointers are extended to smart pointers: - nullptr used as return value or argument for non-null functions (`-Wnonnull`) - assigning or initializing nonnull variables with nullable values (`-Wnullable-to-nonnull-conversion`) It doesn't implicitly add these attributes based on the assume_nonnull pragma, nor warn on missing attributes where the pragma would apply them. I'm not confident that the pragma's current behavior will work well for C++ (where type-based metaprogramming is much more common than C/ObjC). We'd like to revisit this once we have more implementation experience. Support can be detected as `__has_feature(nullability_on_gsl_pointer)`. This is needed for back-compatibility, as previously clang would issue a hard error when _Nullable appears on a smart pointer. UBSan's `-fsanitize=nullability` will not check smart-pointer types. It can be made to do so by synthesizing calls to `operator bool`, but that's left for future work.
1 parent 770fd38 commit 77baccc

File tree

10 files changed

+127
-18
lines changed

10 files changed

+127
-18
lines changed

clang/include/clang/Basic/AttrDocs.td

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4096,6 +4096,10 @@ non-underscored keywords. For example:
40964096
@property (assign, nullable) NSView *superview;
40974097
@property (readonly, nonnull) NSArray *subviews;
40984098
@end
4099+
4100+
As well as built-in pointer types, ithe nullability attributes can be attached
4101+
to the standard C++ pointer types ``std::unique_ptr`` and ``std::shared_ptr``,
4102+
as well as C++ classes marked with the ``gsl::Pointer`` attribute.
40994103
}];
41004104
}
41014105

clang/include/clang/Basic/Features.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ EXTENSION(define_target_os_macros,
9494
FEATURE(enumerator_attributes, true)
9595
FEATURE(nullability, true)
9696
FEATURE(nullability_on_arrays, true)
97+
FEATURE(nullability_on_gsl_pointer, true)
9798
FEATURE(nullability_nullable_result, true)
9899
FEATURE(memory_sanitizer,
99100
LangOpts.Sanitize.hasOneOf(SanitizerKind::Memory |

clang/lib/AST/Type.cpp

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
#include "llvm/ADT/ArrayRef.h"
4545
#include "llvm/ADT/FoldingSet.h"
4646
#include "llvm/ADT/SmallVector.h"
47+
#include "llvm/ADT/StringSwitch.h"
4748
#include "llvm/Support/Casting.h"
4849
#include "llvm/Support/ErrorHandling.h"
4950
#include "llvm/Support/MathExtras.h"
@@ -4524,6 +4525,26 @@ std::optional<NullabilityKind> Type::getNullability() const {
45244525
return std::nullopt;
45254526
}
45264527

4528+
// Smart pointers are well-known stdlib types or marked with [[gsl::Pointer]].
4529+
static bool classCanHaveNullability(CXXRecordDecl *CRD) {
4530+
if (!CRD)
4531+
return false;
4532+
if (CRD->hasAttr<PointerAttr>())
4533+
return true;
4534+
if (auto *CTSD = llvm::dyn_cast<ClassTemplateSpecializationDecl>(CRD))
4535+
if (CTSD->getSpecializedTemplate()
4536+
->getTemplatedDecl()
4537+
->hasAttr<PointerAttr>())
4538+
return true;
4539+
if (CRD->isInStdNamespace())
4540+
if (auto *II = CRD->getIdentifier())
4541+
return llvm::StringSwitch<bool>(II->getName())
4542+
.Cases("auto_ptr", "inout_ptr_t", "out_ptr_t", "shared_ptr",
4543+
"unique_ptr", true)
4544+
.Default(false);
4545+
return false;
4546+
}
4547+
45274548
bool Type::canHaveNullability(bool ResultIfUnknown) const {
45284549
QualType type = getCanonicalTypeInternal();
45294550

@@ -4556,16 +4577,15 @@ bool Type::canHaveNullability(bool ResultIfUnknown) const {
45564577
case Type::Auto:
45574578
return ResultIfUnknown;
45584579

4559-
// Dependent template specializations can instantiate to pointer
4560-
// types unless they're known to be specializations of a class
4561-
// template.
4580+
// Dependent template specializations could instantiate to pointer types.
45624581
case Type::TemplateSpecialization:
4563-
if (TemplateDecl *templateDecl
4564-
= cast<TemplateSpecializationType>(type.getTypePtr())
4565-
->getTemplateName().getAsTemplateDecl()) {
4566-
if (isa<ClassTemplateDecl>(templateDecl))
4567-
return false;
4568-
}
4582+
// If it's a known class template, we can already check if it's a pointer.
4583+
if (TemplateDecl *templateDecl =
4584+
cast<TemplateSpecializationType>(type.getTypePtr())
4585+
->getTemplateName()
4586+
.getAsTemplateDecl())
4587+
if (auto *CTD = dyn_cast<ClassTemplateDecl>(templateDecl))
4588+
return classCanHaveNullability(CTD->getTemplatedDecl());
45694589
return ResultIfUnknown;
45704590

45714591
case Type::Builtin:
@@ -4622,6 +4642,10 @@ bool Type::canHaveNullability(bool ResultIfUnknown) const {
46224642
}
46234643
llvm_unreachable("unknown builtin type");
46244644

4645+
case Type::Record:
4646+
return classCanHaveNullability(
4647+
cast<RecordType>(type)->getAsCXXRecordDecl());
4648+
46254649
// Non-pointer types.
46264650
case Type::Complex:
46274651
case Type::LValueReference:
@@ -4639,7 +4663,6 @@ bool Type::canHaveNullability(bool ResultIfUnknown) const {
46394663
case Type::DependentAddressSpace:
46404664
case Type::FunctionProto:
46414665
case Type::FunctionNoProto:
4642-
case Type::Record:
46434666
case Type::DeducedTemplateSpecialization:
46444667
case Type::Enum:
46454668
case Type::InjectedClassName:

clang/lib/CodeGen/CGCall.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4373,7 +4373,8 @@ void CodeGenFunction::EmitNonNullArgCheck(RValue RV, QualType ArgType,
43734373
NNAttr = getNonNullAttr(AC.getDecl(), PVD, ArgType, ArgNo);
43744374

43754375
bool CanCheckNullability = false;
4376-
if (SanOpts.has(SanitizerKind::NullabilityArg) && !NNAttr && PVD) {
4376+
if (SanOpts.has(SanitizerKind::NullabilityArg) && !NNAttr && PVD &&
4377+
!PVD->getType()->isRecordType()) {
43774378
auto Nullability = PVD->getType()->getNullability();
43784379
CanCheckNullability = Nullability &&
43794380
*Nullability == NullabilityKind::NonNull &&

clang/lib/CodeGen/CodeGenFunction.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -974,7 +974,8 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
974974
// return value. Initialize the flag to 'true' and refine it in EmitParmDecl.
975975
if (SanOpts.has(SanitizerKind::NullabilityReturn)) {
976976
auto Nullability = FnRetTy->getNullability();
977-
if (Nullability && *Nullability == NullabilityKind::NonNull) {
977+
if (Nullability && *Nullability == NullabilityKind::NonNull &&
978+
!FnRetTy->isRecordType()) {
978979
if (!(SanOpts.has(SanitizerKind::ReturnsNonnullAttribute) &&
979980
CurCodeDecl && CurCodeDecl->getAttr<ReturnsNonNullAttr>()))
980981
RetValNullabilityPrecondition =

clang/lib/Sema/SemaChecking.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "clang/AST/ExprObjC.h"
2828
#include "clang/AST/ExprOpenMP.h"
2929
#include "clang/AST/FormatString.h"
30+
#include "clang/AST/IgnoreExpr.h"
3031
#include "clang/AST/NSAPI.h"
3132
#include "clang/AST/NonTrivialTypeVisitor.h"
3233
#include "clang/AST/OperationKinds.h"
@@ -7154,6 +7155,14 @@ bool Sema::getFormatStringInfo(const FormatAttr *Format, bool IsCXXMember,
71547155
///
71557156
/// Returns true if the value evaluates to null.
71567157
static bool CheckNonNullExpr(Sema &S, const Expr *Expr) {
7158+
// Treat (smart) pointers constructed from nullptr as null, whether we can
7159+
// const-evaluate them or not.
7160+
// This must happen first: the smart pointer expr might have _Nonnull type!
7161+
if (isa<CXXNullPtrLiteralExpr>(
7162+
IgnoreExprNodes(Expr, IgnoreImplicitAsWrittenSingleStep,
7163+
IgnoreElidableImplicitConstructorSingleStep)))
7164+
return true;
7165+
71577166
// If the expression has non-null type, it doesn't evaluate to null.
71587167
if (auto nullability = Expr->IgnoreImplicit()->getType()->getNullability()) {
71597168
if (*nullability == NullabilityKind::NonNull)

clang/lib/Sema/SemaInit.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7050,6 +7050,11 @@ PerformConstructorInitialization(Sema &S,
70507050
hasCopyOrMoveCtorParam(S.Context,
70517051
getConstructorInfo(Step.Function.FoundDecl));
70527052

7053+
// A smart pointer constructed from a null pointer is almost certainly null.
7054+
if (NumArgs == 1 && !Kind.isExplicitCast())
7055+
S.diagnoseNullableToNonnullConversion(Entity.getType(), Args.front()->getType(),
7056+
Kind.getLocation());
7057+
70537058
// Determine the arguments required to actually perform the constructor
70547059
// call.
70557060
if (S.CompleteConstructorCall(Constructor, Step.Type, Args, Loc,

clang/lib/Sema/SemaOverload.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14706,6 +14706,13 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
1470614706
}
1470714707
}
1470814708

14709+
// Check for nonnull = nullable.
14710+
// This won't be caught in the arg's initialization: the parameter to
14711+
// the assignment operator is not marked nonnull.
14712+
if (Op == OO_Equal)
14713+
diagnoseNullableToNonnullConversion(Args[0]->getType(),
14714+
Args[1]->getType(), OpLoc);
14715+
1470914716
// Convert the arguments.
1471014717
if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(FnDecl)) {
1471114718
// Best->Access is only meaningful for class members.

clang/lib/Sema/SemaType.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4705,6 +4705,18 @@ static bool DiagnoseMultipleAddrSpaceAttributes(Sema &S, LangAS ASOld,
47054705
return false;
47064706
}
47074707

4708+
// Whether this is a type broadly expected to have nullability attached.
4709+
// These types are affected by `#pragma assume_nonnull`, and missing nullability
4710+
// will be diagnosed with -Wnullability-completeness.
4711+
static bool shouldHaveNullability(QualType T) {
4712+
return T->canHaveNullability(/*ResultIfUnknown=*/false) &&
4713+
// For now, do not infer/require nullability on C++ smart pointers.
4714+
// It's unclear whether the pragma's behavior is useful for C++.
4715+
// e.g. treating type-aliases and template-type-parameters differently
4716+
// from types of declarations can be surprising.
4717+
!isa<RecordType>(T);
4718+
}
4719+
47084720
static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
47094721
QualType declSpecType,
47104722
TypeSourceInfo *TInfo) {
@@ -4823,8 +4835,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
48234835
// inner pointers.
48244836
complainAboutMissingNullability = CAMN_InnerPointers;
48254837

4826-
if (T->canHaveNullability(/*ResultIfUnknown*/ false) &&
4827-
!T->getNullability()) {
4838+
if (shouldHaveNullability(T) && !T->getNullability()) {
48284839
// Note that we allow but don't require nullability on dependent types.
48294840
++NumPointersRemaining;
48304841
}
@@ -5047,8 +5058,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
50475058
// If the type itself could have nullability but does not, infer pointer
50485059
// nullability and perform consistency checking.
50495060
if (S.CodeSynthesisContexts.empty()) {
5050-
if (T->canHaveNullability(/*ResultIfUnknown*/ false) &&
5051-
!T->getNullability()) {
5061+
if (shouldHaveNullability(T) && !T->getNullability()) {
50525062
if (isVaList(T)) {
50535063
// Record that we've seen a pointer, but do nothing else.
50545064
if (NumPointersRemaining > 0)

clang/test/SemaCXX/nullability.cpp

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
#else
55
# error nullability feature should be defined
66
#endif
7+
#if __has_feature(nullability_on_gsl_pointer)
8+
#else
9+
# error smart-pointer feature should be defined
10+
#endif
711

812
#include "nullability-completeness.h"
913

@@ -27,6 +31,7 @@ template<typename T>
2731
struct AddNonNull {
2832
typedef _Nonnull T type; // expected-error{{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'int'}}
2933
// expected-error@-1{{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'std::nullptr_t'}}
34+
// expected-error@-2{{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'NotPtr'}}
3035
};
3136

3237
typedef AddNonNull<int *>::type nonnull_int_ptr_1;
@@ -35,6 +40,23 @@ typedef AddNonNull<nullptr_t>::type nonnull_int_ptr_3; // expected-note{{in inst
3540

3641
typedef AddNonNull<int>::type nonnull_non_pointer_1; // expected-note{{in instantiation of template class 'AddNonNull<int>' requested here}}
3742

43+
// Nullability on C++ class types (smart pointers).
44+
struct NotPtr{};
45+
typedef AddNonNull<NotPtr>::type nonnull_non_pointer_2; // expected-note{{in instantiation}}
46+
struct [[gsl::Pointer]] SmartPtr{
47+
SmartPtr();
48+
SmartPtr(nullptr_t);
49+
SmartPtr(const SmartPtr&);
50+
SmartPtr(SmartPtr&&);
51+
SmartPtr &operator=(const SmartPtr&);
52+
SmartPtr &operator=(SmartPtr&&);
53+
};
54+
typedef AddNonNull<SmartPtr>::type nonnull_smart_pointer_1;
55+
template<class> struct [[gsl::Pointer]] SmartPtrTemplate{};
56+
typedef AddNonNull<SmartPtrTemplate<int>>::type nonnull_smart_pointer_2;
57+
namespace std { inline namespace __1 { template <class> class unique_ptr {}; } }
58+
typedef AddNonNull<std::unique_ptr<int>>::type nonnull_smart_pointer_3;
59+
3860
// Non-null checking within a template.
3961
template<typename T>
4062
struct AddNonNull2 {
@@ -54,13 +76,16 @@ void (*& accepts_nonnull_2)(_Nonnull int *ptr) = accepts_nonnull_1;
5476
void (X::* accepts_nonnull_3)(_Nonnull int *ptr);
5577
void accepts_nonnull_4(_Nonnull int *ptr);
5678
void (&accepts_nonnull_5)(_Nonnull int *ptr) = accepts_nonnull_4;
79+
void accepts_nonnull_6(SmartPtr _Nonnull);
5780

5881
void test_accepts_nonnull_null_pointer_literal(X *x) {
5982
accepts_nonnull_1(0); // expected-warning{{null passed to a callee that requires a non-null argument}}
6083
accepts_nonnull_2(0); // expected-warning{{null passed to a callee that requires a non-null argument}}
6184
(x->*accepts_nonnull_3)(0); // expected-warning{{null passed to a callee that requires a non-null argument}}
6285
accepts_nonnull_4(0); // expected-warning{{null passed to a callee that requires a non-null argument}}
6386
accepts_nonnull_5(0); // expected-warning{{null passed to a callee that requires a non-null argument}}
87+
88+
accepts_nonnull_6(nullptr); // expected-warning{{null passed to a callee that requires a non-null argument}}
6489
}
6590

6691
template<void FP(_Nonnull int*)>
@@ -71,6 +96,7 @@ void test_accepts_nonnull_null_pointer_literal_template() {
7196
template void test_accepts_nonnull_null_pointer_literal_template<&accepts_nonnull_4>(); // expected-note{{instantiation of function template specialization}}
7297

7398
void TakeNonnull(void *_Nonnull);
99+
void TakeSmartNonnull(SmartPtr _Nonnull);
74100
// Check different forms of assignment to a nonull type from a nullable one.
75101
void AssignAndInitNonNull() {
76102
void *_Nullable nullable;
@@ -81,12 +107,26 @@ void AssignAndInitNonNull() {
81107
void *_Nonnull nonnull;
82108
nonnull = nullable; // expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull'}}
83109
nonnull = {nullable}; // expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull'}}
84-
85110
TakeNonnull(nullable); //expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull}}
86111
TakeNonnull(nonnull); // OK
112+
nonnull = (void *_Nonnull)nullable; // explicit cast OK
113+
114+
SmartPtr _Nullable s_nullable;
115+
SmartPtr _Nonnull s(s_nullable); // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}}
116+
SmartPtr _Nonnull s2{s_nullable}; // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}}
117+
SmartPtr _Nonnull s3 = {s_nullable}; // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}}
118+
SmartPtr _Nonnull s4 = s_nullable; // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}}
119+
SmartPtr _Nonnull s_nonnull;
120+
s_nonnull = s_nullable; // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}}
121+
s_nonnull = {s_nullable}; // no warning here - might be nice?
122+
TakeSmartNonnull(s_nullable); //expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull}}
123+
TakeSmartNonnull(s_nonnull); // OK
124+
s_nonnull = (SmartPtr _Nonnull)s_nullable; // explicit cast OK
125+
s_nonnull = static_cast<SmartPtr _Nonnull>(s_nullable); // explicit cast OK
87126
}
88127

89128
void *_Nullable ReturnNullable();
129+
SmartPtr _Nullable ReturnSmartNullable();
90130

91131
void AssignAndInitNonNullFromFn() {
92132
void *_Nonnull p(ReturnNullable()); // expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull'}}
@@ -96,8 +136,16 @@ void AssignAndInitNonNullFromFn() {
96136
void *_Nonnull nonnull;
97137
nonnull = ReturnNullable(); // expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull'}}
98138
nonnull = {ReturnNullable()}; // expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull'}}
99-
100139
TakeNonnull(ReturnNullable()); //expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull}}
140+
141+
SmartPtr _Nonnull s(ReturnSmartNullable()); // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}}
142+
SmartPtr _Nonnull s2{ReturnSmartNullable()}; // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}}
143+
SmartPtr _Nonnull s3 = {ReturnSmartNullable()}; // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}}
144+
SmartPtr _Nonnull s4 = ReturnSmartNullable(); // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}}
145+
SmartPtr _Nonnull s_nonnull;
146+
s_nonnull = ReturnSmartNullable(); // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}}
147+
s_nonnull = {ReturnSmartNullable()};
148+
TakeSmartNonnull(ReturnSmartNullable()); //expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull}}
101149
}
102150

103151
void ConditionalExpr(bool c) {

0 commit comments

Comments
 (0)