This is an archive of the discontinued LLVM Phabricator instance.

[llvm] Check availability for os_signpost
ClosedPublic

Authored by JDevlieghere on Feb 24 2021, 12:36 PM.

Details

Summary

Add availability checks to the os_signpost code so this can be enabled with an older deployment target.

Diff Detail

Event Timeline

JDevlieghere created this revision.Feb 24 2021, 12:36 PM
JDevlieghere requested review of this revision.Feb 24 2021, 12:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 24 2021, 12:36 PM
dsanders accepted this revision.Feb 24 2021, 4:05 PM

LGTM with a couple nitpicks

llvm/lib/Support/Signposts.cpp
54

We might want to wrap the version list in a macro so we don't have to dupe it in each function. I'm thinking something along the lines of:

#define SIGNPOSTS_AVAILABLE() __builtin_available(macos 10.14, iOS 12, tvOS 12, watchOS 5, *)
57

Do we still need to insert it when signposts aren't available? The call to getSignpostForObject() isn't reachable in that case

72–76

Is this needed? isEnabled() will be false but I can believe the warning isn't smart enough to see that.

This revision is now accepted and ready to land.Feb 24 2021, 4:05 PM
JDevlieghere marked 3 inline comments as done.Feb 24 2021, 4:22 PM
JDevlieghere added inline comments.
llvm/lib/Support/Signposts.cpp
72–76

Yep, in both cases the compiler isn't smart enough.

This revision was automatically updated to reflect the committed changes.
JDevlieghere marked an inline comment as done.