This is an archive of the discontinued LLVM Phabricator instance.

[Sema] Improve diagnostic message for unavailable c++17 aligned allocation functions
ClosedPublic

Authored by ahatanak on Jul 17 2017, 5:07 PM.

Details

Summary

This changes the error message Sema prints when an unavailable c++17 aligned allocation function is selected.

Original message: "... possibly unavailable on x86_64-apple-macos10.12"
New message: "... only available on macosx 10.13 or newer"

This is a follow-up to r306722.

Diff Detail

Repository
rL LLVM

Event Timeline

ahatanak created this revision.Jul 17 2017, 5:07 PM
arphaman added inline comments.Jul 18 2017, 6:02 AM
include/clang/Basic/AlignedAllocation.h
25 ↗(On Diff #106982)

Redundant static?

31 ↗(On Diff #106982)

You can drop the last 0 here and two last zeros down below. This will also make the diagnostics less verbose, i.e. instead of iOS 11.0.0 we will show iOS 11.

42 ↗(On Diff #106982)

// end namespace clang

44 ↗(On Diff #106982)

NIT: Missing // LLVM_CLANG_BASIC_ALIGNED_ALLOCATION_H comment

lib/Sema/SemaExprCXX.cpp
1667 ↗(On Diff #106982)

Can you use AvailabilityAttr::getPlatformNameSourceSpelling instead to be consisted with our unguarded availability warnings?
e.g.:

AvailabilityAttr::getPlatformNameSourceSpelling(S.getASTContext().getTargetInfo().getPlatformName())

This will ensure that the diagnostics will use names like 'macOS' instead of 'macosx'.

aaron.ballman added inline comments.
include/clang/Basic/AlignedAllocation.h
12 ↗(On Diff #106982)

c++17 -> C++17

ahatanak updated this revision to Diff 107140.Jul 18 2017, 11:10 AM
ahatanak marked 6 inline comments as done.

Address review comments.

arphaman accepted this revision.Jul 19 2017, 2:39 AM

LGTM, Thanks.

This revision is now accepted and ready to land.Jul 19 2017, 2:39 AM
This revision was automatically updated to reflect the committed changes.