Skip to content

Revert "Reapply "[clang][nullability] allow _Nonnull etc on nullable class types (#82705)"" #87041

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -253,21 +253,6 @@ Attribute Changes in Clang
added a new extension query ``__has_extension(swiftcc)`` corresponding to the
``__attribute__((swiftcc))`` attribute.

- The ``_Nullable`` and ``_Nonnull`` family of type attributes can now apply
to certain C++ class types, such as smart pointers:
``void useObject(std::unique_ptr<Object> _Nonnull obj);``.

This works for standard library types including ``unique_ptr``, ``shared_ptr``,
and ``function``. See
`the attribute reference documentation <https://llvm.org/docs/AttributeReference.html#nullability-attributes>`_
for the full list.

- The ``_Nullable`` attribute can be applied to C++ class declarations:
``template <class T> class _Nullable MySmartPointer {};``.

This allows the ``_Nullable`` and ``_Nonnull`` family of type attributes to
apply to this class.

Improvements to Clang's diagnostics
-----------------------------------
- Clang now applies syntax highlighting to the code snippets it
Expand Down
3 changes: 1 addition & 2 deletions clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -2178,10 +2178,9 @@ def TypeNonNull : TypeAttr {
let Documentation = [TypeNonNullDocs];
}

def TypeNullable : DeclOrTypeAttr {
def TypeNullable : TypeAttr {
let Spellings = [CustomKeyword<"_Nullable">];
let Documentation = [TypeNullableDocs];
// let Subjects = SubjectList<[CXXRecord], ErrorDiag>;
}

def TypeNullableResult : TypeAttr {
Expand Down
25 changes: 0 additions & 25 deletions clang/include/clang/Basic/AttrDocs.td
Original file line number Diff line number Diff line change
Expand Up @@ -4151,20 +4151,6 @@ non-underscored keywords. For example:
@property (assign, nullable) NSView *superview;
@property (readonly, nonnull) NSArray *subviews;
@end

As well as built-in pointer types, the nullability attributes can be attached
to C++ classes marked with the ``_Nullable`` attribute.

The following C++ standard library types are considered nullable:
``unique_ptr``, ``shared_ptr``, ``auto_ptr``, ``exception_ptr``, ``function``,
``move_only_function`` and ``coroutine_handle``.

Types should be marked nullable only where the type itself leaves nullability
ambiguous. For example, ``std::optional`` is not marked ``_Nullable``, because
``optional<int> _Nullable`` is redundant and ``optional<int> _Nonnull`` is
not a useful type. ``std::weak_ptr`` is not nullable, because its nullability
can change with no visible modification, so static annotation is unlikely to be
unhelpful.
}];
}

Expand Down Expand Up @@ -4199,17 +4185,6 @@ The ``_Nullable`` nullability qualifier indicates that a value of the
int fetch_or_zero(int * _Nullable ptr);

a caller of ``fetch_or_zero`` can provide null.

The ``_Nullable`` attribute on classes indicates that the given class can
represent null values, and so the ``_Nullable``, ``_Nonnull`` etc qualifiers
make sense for this type. For example:

.. code-block:: c

class _Nullable ArenaPointer { ... };

ArenaPointer _Nonnull x = ...;
ArenaPointer _Nullable y = nullptr;
}];
}

Expand Down
1 change: 0 additions & 1 deletion clang/include/clang/Basic/Features.def
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ EXTENSION(define_target_os_macros,
FEATURE(enumerator_attributes, true)
FEATURE(nullability, true)
FEATURE(nullability_on_arrays, true)
FEATURE(nullability_on_classes, true)
FEATURE(nullability_nullable_result, true)
FEATURE(memory_sanitizer,
LangOpts.Sanitize.hasOneOf(SanitizerKind::Memory |
Expand Down
1 change: 0 additions & 1 deletion clang/include/clang/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -3014,7 +3014,6 @@ class Parser : public CodeCompletionHandler {
void DiagnoseAndSkipExtendedMicrosoftTypeAttributes();
SourceLocation SkipExtendedMicrosoftTypeAttributes();
void ParseMicrosoftInheritanceClassAttributes(ParsedAttributes &attrs);
void ParseNullabilityClassAttributes(ParsedAttributes &attrs);
void ParseBorlandTypeAttributes(ParsedAttributes &attrs);
void ParseOpenCLKernelAttributes(ParsedAttributes &attrs);
void ParseOpenCLQualifiers(ParsedAttributes &Attrs);
Expand Down
3 changes: 0 additions & 3 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -1655,9 +1655,6 @@ class Sema final {
/// Add [[gsl::Pointer]] attributes for std:: types.
void inferGslPointerAttribute(TypedefNameDecl *TD);

/// Add _Nullable attributes for std:: types.
void inferNullableClassAttribute(CXXRecordDecl *CRD);

enum PragmaOptionsAlignKind {
POAK_Native, // #pragma options align=native
POAK_Natural, // #pragma options align=natural
Expand Down
29 changes: 10 additions & 19 deletions clang/lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4642,15 +4642,16 @@ bool Type::canHaveNullability(bool ResultIfUnknown) const {
case Type::Auto:
return ResultIfUnknown;

// Dependent template specializations could instantiate to pointer types.
// Dependent template specializations can instantiate to pointer
// types unless they're known to be specializations of a class
// template.
case Type::TemplateSpecialization:
// If it's a known class template, we can already check if it's nullable.
if (TemplateDecl *templateDecl =
cast<TemplateSpecializationType>(type.getTypePtr())
->getTemplateName()
.getAsTemplateDecl())
if (auto *CTD = dyn_cast<ClassTemplateDecl>(templateDecl))
return CTD->getTemplatedDecl()->hasAttr<TypeNullableAttr>();
if (TemplateDecl *templateDecl
= cast<TemplateSpecializationType>(type.getTypePtr())
->getTemplateName().getAsTemplateDecl()) {
if (isa<ClassTemplateDecl>(templateDecl))
return false;
}
return ResultIfUnknown;

case Type::Builtin:
Expand Down Expand Up @@ -4707,17 +4708,6 @@ bool Type::canHaveNullability(bool ResultIfUnknown) const {
}
llvm_unreachable("unknown builtin type");

case Type::Record: {
const RecordDecl *RD = cast<RecordType>(type)->getDecl();
// For template specializations, look only at primary template attributes.
// This is a consistent regardless of whether the instantiation is known.
if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
return CTSD->getSpecializedTemplate()
->getTemplatedDecl()
->hasAttr<TypeNullableAttr>();
return RD->hasAttr<TypeNullableAttr>();
}

// Non-pointer types.
case Type::Complex:
case Type::LValueReference:
Expand All @@ -4735,6 +4725,7 @@ bool Type::canHaveNullability(bool ResultIfUnknown) const {
case Type::DependentAddressSpace:
case Type::FunctionProto:
case Type::FunctionNoProto:
case Type::Record:
case Type::DeducedTemplateSpecialization:
case Type::Enum:
case Type::InjectedClassName:
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/CodeGen/CGCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4379,8 +4379,7 @@ void CodeGenFunction::EmitNonNullArgCheck(RValue RV, QualType ArgType,
NNAttr = getNonNullAttr(AC.getDecl(), PVD, ArgType, ArgNo);

bool CanCheckNullability = false;
if (SanOpts.has(SanitizerKind::NullabilityArg) && !NNAttr && PVD &&
!PVD->getType()->isRecordType()) {
if (SanOpts.has(SanitizerKind::NullabilityArg) && !NNAttr && PVD) {
auto Nullability = PVD->getType()->getNullability();
CanCheckNullability = Nullability &&
*Nullability == NullabilityKind::NonNull &&
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/CodeGen/CodeGenFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -989,8 +989,7 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
// return value. Initialize the flag to 'true' and refine it in EmitParmDecl.
if (SanOpts.has(SanitizerKind::NullabilityReturn)) {
auto Nullability = FnRetTy->getNullability();
if (Nullability && *Nullability == NullabilityKind::NonNull &&
!FnRetTy->isRecordType()) {
if (Nullability && *Nullability == NullabilityKind::NonNull) {
if (!(SanOpts.has(SanitizerKind::ReturnsNonnullAttribute) &&
CurCodeDecl && CurCodeDecl->getAttr<ReturnsNonNullAttr>()))
RetValNullabilityPrecondition =
Expand Down
33 changes: 9 additions & 24 deletions clang/lib/Parse/ParseDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1502,15 +1502,6 @@ void Parser::ParseMicrosoftInheritanceClassAttributes(ParsedAttributes &attrs) {
}
}

void Parser::ParseNullabilityClassAttributes(ParsedAttributes &attrs) {
while (Tok.is(tok::kw__Nullable)) {
IdentifierInfo *AttrName = Tok.getIdentifierInfo();
auto Kind = Tok.getKind();
SourceLocation AttrNameLoc = ConsumeToken();
attrs.addNew(AttrName, AttrNameLoc, nullptr, AttrNameLoc, nullptr, 0, Kind);
}
}

/// Determine whether the following tokens are valid after a type-specifier
/// which could be a standalone declaration. This will conservatively return
/// true if there's any doubt, and is appropriate for insert-';' fixits.
Expand Down Expand Up @@ -1692,21 +1683,15 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,

ParsedAttributes attrs(AttrFactory);
// If attributes exist after tag, parse them.
for (;;) {
MaybeParseAttributes(PAKM_CXX11 | PAKM_Declspec | PAKM_GNU, attrs);
// Parse inheritance specifiers.
if (Tok.isOneOf(tok::kw___single_inheritance,
tok::kw___multiple_inheritance,
tok::kw___virtual_inheritance)) {
ParseMicrosoftInheritanceClassAttributes(attrs);
continue;
}
if (Tok.is(tok::kw__Nullable)) {
ParseNullabilityClassAttributes(attrs);
continue;
}
break;
}
MaybeParseAttributes(PAKM_CXX11 | PAKM_Declspec | PAKM_GNU, attrs);

// Parse inheritance specifiers.
if (Tok.isOneOf(tok::kw___single_inheritance, tok::kw___multiple_inheritance,
tok::kw___virtual_inheritance))
ParseMicrosoftInheritanceClassAttributes(attrs);

// Allow attributes to precede or succeed the inheritance specifiers.
MaybeParseAttributes(PAKM_CXX11 | PAKM_Declspec | PAKM_GNU, attrs);

// Source location used by FIXIT to insert misplaced
// C++11 attributes
Expand Down
12 changes: 0 additions & 12 deletions clang/lib/Sema/SemaAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,18 +215,6 @@ void Sema::inferGslOwnerPointerAttribute(CXXRecordDecl *Record) {
inferGslPointerAttribute(Record, Record);
}

void Sema::inferNullableClassAttribute(CXXRecordDecl *CRD) {
static llvm::StringSet<> Nullable{
"auto_ptr", "shared_ptr", "unique_ptr", "exception_ptr",
"coroutine_handle", "function", "move_only_function",
};

if (CRD->isInStdNamespace() && Nullable.count(CRD->getName()) &&
!CRD->hasAttr<TypeNullableAttr>())
for (Decl *Redecl : CRD->redecls())
Redecl->addAttr(TypeNullableAttr::CreateImplicit(Context));
}

void Sema::ActOnPragmaOptionsAlign(PragmaOptionsAlignKind Kind,
SourceLocation PragmaLoc) {
PragmaMsStackAction Action = Sema::PSK_Reset;
Expand Down
9 changes: 0 additions & 9 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
#include "clang/AST/ExprObjC.h"
#include "clang/AST/ExprOpenMP.h"
#include "clang/AST/FormatString.h"
#include "clang/AST/IgnoreExpr.h"
#include "clang/AST/NSAPI.h"
#include "clang/AST/NonTrivialTypeVisitor.h"
#include "clang/AST/OperationKinds.h"
Expand Down Expand Up @@ -7610,14 +7609,6 @@ bool Sema::getFormatStringInfo(const FormatAttr *Format, bool IsCXXMember,
///
/// Returns true if the value evaluates to null.
static bool CheckNonNullExpr(Sema &S, const Expr *Expr) {
// Treat (smart) pointers constructed from nullptr as null, whether we can
// const-evaluate them or not.
// This must happen first: the smart pointer expr might have _Nonnull type!
if (isa<CXXNullPtrLiteralExpr>(
IgnoreExprNodes(Expr, IgnoreImplicitAsWrittenSingleStep,
IgnoreElidableImplicitConstructorSingleStep)))
return true;

// If the expression has non-null type, it doesn't evaluate to null.
if (auto nullability = Expr->IgnoreImplicit()->getType()->getNullability()) {
if (*nullability == NullabilityKind::NonNull)
Expand Down
4 changes: 1 addition & 3 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18317,10 +18317,8 @@ Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc,
if (PrevDecl)
mergeDeclAttributes(New, PrevDecl);

if (auto *CXXRD = dyn_cast<CXXRecordDecl>(New)) {
if (auto *CXXRD = dyn_cast<CXXRecordDecl>(New))
inferGslOwnerPointerAttribute(CXXRD);
inferNullableClassAttribute(CXXRD);
}

// If there's a #pragma GCC visibility in scope, set the visibility of this
// record.
Expand Down
18 changes: 0 additions & 18 deletions clang/lib/Sema/SemaDeclAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5982,20 +5982,6 @@ static void handleBuiltinAliasAttr(Sema &S, Decl *D,
D->addAttr(::new (S.Context) BuiltinAliasAttr(S.Context, AL, Ident));
}

static void handleNullableTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
if (AL.isUsedAsTypeAttr())
return;

if (auto *CRD = dyn_cast<CXXRecordDecl>(D);
!CRD || !(CRD->isClass() || CRD->isStruct())) {
S.Diag(AL.getRange().getBegin(), diag::err_attribute_wrong_decl_type_str)
<< AL << AL.isRegularKeywordAttribute() << "classes";
return;
}

handleSimpleAttribute<TypeNullableAttr>(S, D, AL);
}

static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
if (!AL.hasParsedType()) {
S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 1;
Expand Down Expand Up @@ -9947,10 +9933,6 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
case ParsedAttr::AT_UsingIfExists:
handleSimpleAttribute<UsingIfExistsAttr>(S, D, AL);
break;

case ParsedAttr::AT_TypeNullable:
handleNullableTypeAttr(S, D, AL);
break;
}
}

Expand Down
5 changes: 0 additions & 5 deletions clang/lib/Sema/SemaInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7079,11 +7079,6 @@ PerformConstructorInitialization(Sema &S,
hasCopyOrMoveCtorParam(S.Context,
getConstructorInfo(Step.Function.FoundDecl));

// A smart pointer constructed from a nullable pointer is nullable.
if (NumArgs == 1 && !Kind.isExplicitCast())
S.diagnoseNullableToNonnullConversion(
Entity.getType(), Args.front()->getType(), Kind.getLocation());

// Determine the arguments required to actually perform the constructor
// call.
if (S.CompleteConstructorCall(Constructor, Step.Type, Args, Loc,
Expand Down
7 changes: 0 additions & 7 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14811,13 +14811,6 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
}
}

// Check for nonnull = nullable.
// This won't be caught in the arg's initialization: the parameter to
// the assignment operator is not marked nonnull.
if (Op == OO_Equal)
diagnoseNullableToNonnullConversion(Args[0]->getType(),
Args[1]->getType(), OpLoc);

// Convert the arguments.
if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(FnDecl)) {
// Best->Access is only meaningful for class members.
Expand Down
1 change: 0 additions & 1 deletion clang/lib/Sema/SemaTemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2171,7 +2171,6 @@ DeclResult Sema::CheckClassTemplate(

AddPushedVisibilityAttribute(NewClass);
inferGslOwnerPointerAttribute(NewClass);
inferNullableClassAttribute(NewClass);

if (TUK != TUK_Friend) {
// Per C++ [basic.scope.temp]p2, skip the template parameter scopes.
Expand Down
18 changes: 4 additions & 14 deletions clang/lib/Sema/SemaType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4717,18 +4717,6 @@ static bool DiagnoseMultipleAddrSpaceAttributes(Sema &S, LangAS ASOld,
return false;
}

// Whether this is a type broadly expected to have nullability attached.
// These types are affected by `#pragma assume_nonnull`, and missing nullability
// will be diagnosed with -Wnullability-completeness.
static bool shouldHaveNullability(QualType T) {
return T->canHaveNullability(/*ResultIfUnknown=*/false) &&
// For now, do not infer/require nullability on C++ smart pointers.
// It's unclear whether the pragma's behavior is useful for C++.
// e.g. treating type-aliases and template-type-parameters differently
// from types of declarations can be surprising.
!isa<RecordType>(T->getCanonicalTypeInternal());
}

static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
QualType declSpecType,
TypeSourceInfo *TInfo) {
Expand Down Expand Up @@ -4847,7 +4835,8 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
// inner pointers.
complainAboutMissingNullability = CAMN_InnerPointers;

if (shouldHaveNullability(T) && !T->getNullability()) {
if (T->canHaveNullability(/*ResultIfUnknown*/ false) &&
!T->getNullability()) {
// Note that we allow but don't require nullability on dependent types.
++NumPointersRemaining;
}
Expand Down Expand Up @@ -5070,7 +5059,8 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
// If the type itself could have nullability but does not, infer pointer
// nullability and perform consistency checking.
if (S.CodeSynthesisContexts.empty()) {
if (shouldHaveNullability(T) && !T->getNullability()) {
if (T->canHaveNullability(/*ResultIfUnknown*/ false) &&
!T->getNullability()) {
if (isVaList(T)) {
// Record that we've seen a pointer, but do nothing else.
if (NumPointersRemaining > 0)
Expand Down
2 changes: 0 additions & 2 deletions clang/test/Sema/nullability.c
Original file line number Diff line number Diff line change
Expand Up @@ -248,5 +248,3 @@ void arraysInBlocks(void) {
void (^withTypedefBad)(INTS _Nonnull [2]) = // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'INTS' (aka 'int[4]')}}
^(INTS _Nonnull x[2]) {}; // expected-error {{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'INTS' (aka 'int[4]')}}
}

struct _Nullable NotCplusplusClass {}; // expected-error {{'_Nullable' attribute only applies to classes}}
Loading