This is an archive of the discontinued LLVM Phabricator instance.

[-Wunsafe-buffer-usage] Do not emit fixits for C++ interfaces with C linkage
ClosedPublic

Authored by t-rasmud on Jun 15 2023, 12:09 PM.

Diff Detail

Event Timeline

t-rasmud created this revision.Jun 15 2023, 12:09 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 15 2023, 12:09 PM
t-rasmud requested review of this revision.Jun 15 2023, 12:09 PM
NoQ accepted this revision.Jun 15 2023, 12:49 PM

Nice, LGTM!

I guess it could also be nice to suppress note_safe_buffer_usage_suggestions_disabled in such functions but that's an extremely minor issue.

This revision is now accepted and ready to land.Jun 15 2023, 12:49 PM
ziqingluo-90 added inline comments.Jun 15 2023, 2:42 PM
clang/lib/Analysis/UnsafeBufferUsage.cpp
2146

nitpick: since we already checked the Kind here, can we use cast instead of dyn_cast?

2148

Shall we break the loop?

t-rasmud retitled this revision from [-Wunsafe-buffer-usage][WIP] Do not emit fixits for C++ interfaces with C linkage to [-Wunsafe-buffer-usage] Do not emit fixits for C++ interfaces with C linkage.Jun 15 2023, 3:05 PM
t-rasmud updated this revision to Diff 531928.Jun 15 2023, 3:48 PM

Address comments.

NoQ added inline comments.Jun 15 2023, 4:22 PM
clang/lib/Analysis/UnsafeBufferUsage.cpp
2145–2146

IIRC redecls() picks up on the type, so this can be made even more idiomatic this way (didn't actually try).

t-rasmud updated this revision to Diff 531942.Jun 15 2023, 4:46 PM
t-rasmud marked 2 inline comments as done.

Address feedback.

t-rasmud marked an inline comment as done.Jun 15 2023, 4:46 PM
ziqingluo-90 accepted this revision.Jun 16 2023, 11:08 AM

Looks great to me! I never thought of this case before.

This revision was landed with ongoing or failed builds.Jun 16 2023, 1:28 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 16 2023, 1:29 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript