Skip to content

[-Wunsafe-buffer-usage] Cherrypick fix of FN warning when constructing a span from a C'tor member initializer (#91991) #9537

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

Conversation

ziqingluo-90
Copy link

Respect the [[clang::unsafe_buffer_usage]] attribute for field and constructor initializers (llvm#91991)

CXXCtorInitializers are not statements , but they point to an initializer expression which is. When visiting a FunctionDecl, also walk through any constructor initializers and run the warning checks/matchers against their initializer expressions. This catches warnings for initializing fields and calling other constructors, such as:

struct C {
C(P* Ptr) : AnUnsafeCtor(Ptr) {}
}

Field initializers can be found by traversing CXXDefaultInitExprs. This catches warnings in places such as:

struct C {
P* Ptr;
AnUnsafeCtor U{Ptr};
};

We add tests for explicit construction, for field initialization, base class constructor calls, delegated constructor calls, and aggregate initialization.

Note that aggregate initialization is not fully covered where a field specifies an initializer and it's not overridden in the aggregate initialization, such as in:

struct AggregateViaValueInit {
UnsafeMembers f1;
// FIXME: A construction of this class does initialize the field
// through this initializer, so it should warn. Ideally it should
// also point to where the site of the construction is in
// testAggregateViaValueInit().
UnsafeMembers f2{3};
};

void testAggregateViaValueInit() {
auto A = AggregateViaValueInit();
};

There are 3 tests for different types of aggregate initialization with FIXMEs documenting this future work.

One attempt to fix this involved returning true from MatchDescendantVisitor::shouldVisitImplicitCode(), however, it breaks expectations for field in-class initializers by moving the SourceLocation, possibly to inside the implicit ctor instead of on the line where the field initialization happens.

struct C {
P* Ptr;
AnUnsafeCtor U{Ptr}; // expected-warning{{this is never seen then}}
};

Tests are also added for std::span(ptr, size) constructor being called from a field initializer and a constructor initializer.

Issue llvm#80482

rdar://137999201
(cherry picked from commit a518ed2)

…nstructor initializers (llvm#91991)

CXXCtorInitializers are not statements , but they point to an
initializer expression which is. When visiting a FunctionDecl, also
walk through any constructor initializers and run the warning
checks/matchers against their initializer expressions. This catches
warnings for initializing fields and calling other constructors, such
as:

struct C {
  C(P* Ptr) : AnUnsafeCtor(Ptr) {}
}

Field initializers can be found by traversing CXXDefaultInitExprs. This
catches warnings in places such as:

struct C {
  P* Ptr;
  AnUnsafeCtor U{Ptr};
};

We add tests for explicit construction, for field initialization, base
class constructor calls, delegated constructor calls, and aggregate
initialization.

Note that aggregate initialization is not fully covered where a field
specifies an initializer and it's not overridden in the aggregate initialization,
such as in:

struct AggregateViaValueInit {
    UnsafeMembers f1;
    // FIXME: A construction of this class does initialize the field
    // through this initializer, so it should warn. Ideally it should
    // also point to where the site of the construction is in
    // testAggregateViaValueInit().
    UnsafeMembers f2{3};
};

void testAggregateViaValueInit() {
    auto A = AggregateViaValueInit();
};

There are 3 tests for different types of aggregate initialization with
FIXMEs documenting this future work.

One attempt to fix this involved returning true from
MatchDescendantVisitor::shouldVisitImplicitCode(), however, it breaks expectations
for field in-class initializers by moving the SourceLocation, possibly
to inside the implicit ctor instead of on the line where the field
initialization happens.

struct C {
  P* Ptr;
  AnUnsafeCtor U{Ptr};  // expected-warning{{this is never seen then}}
};

Tests are also added for std::span(ptr, size) constructor being called
from a field initializer and a constructor initializer.

Issue llvm#80482

(cherry picked from commit a518ed2)
@ziqingluo-90 ziqingluo-90 changed the title [-Wunsafe-buffer-usage] Cherrypick fix of FN warning when constructing a span from a c'tor member initializer (#91991) [-Wunsafe-buffer-usage] Cherrypick fix of FN warning when constructing a span from a C'tor member initializer (#91991) Nov 7, 2024
@ziqingluo-90
Copy link
Author

@swift-ci test

@ziqingluo-90 ziqingluo-90 requested review from t-rasmud and malavikasamak and removed request for t-rasmud November 7, 2024 17:48
@ziqingluo-90
Copy link
Author

Still can't add @jkorous-apple to reviewer.

@ziqingluo-90
Copy link
Author

I plan to go ahead and merge this cherry-pick tomorrow, if no objections.

@ziqingluo-90 ziqingluo-90 merged commit 5c0d283 into swiftlang:stable/20240723 Nov 8, 2024
3 checks passed
@ziqingluo-90 ziqingluo-90 deleted the cherrypick-137999201 branch December 19, 2024 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants