search

ned14 / outcome

Provides very lightweight outcome<T> and result<T> (non-Boost edition)
https://ned14.github.io/outcome
Other
704 stars 62 forks source link

Don't use [[likely]] on AppleClang 12 #274

Closed amerry closed 1 year ago

amerry commented 1 year ago

AppleClang 12 doesn't support [[likely]], and warns about an unused attribute. AppleClang 13 does support [[likely]].

amerry commented 1 year ago

It's specifically Xcode 12.5 that this affects, btw. 12.4 and earlier don't pretend to support C++20, and so are caught by the earlier part of the condition.

BurningEnlightenment commented 1 year ago

I wonder why we don't "just" check 

#ifdef __has_cpp_attribute
#if __has_cpp_attribute(likely) && __has_cpp_attribute(unlikely)

according to cppreference __has_cpp_attribute (/P0941) has been implemented on all compilers way before they implemented likely and unlikely support...

ned14 commented 1 year ago

I wonder why we don't "just" check according to cppreference __has_cpp_attribute (/P0941) has been implemented on all compilers way before they implemented likely and unlikely support...

I can tell you why: that macro used to be the proprietary markup, and only had C++20 likely and unlikely added late. I remember it arrived via a PR too, somebody really wanted OUTCOME_TRY to mark the branches as likely and unlikely.

How about we use the proper __has_cpp_attribute() in your PR?

amerry commented 1 year ago

Makes sense. Checking OUTCOME_TRY_LIKELY_IF twice seemed the cleanest way, give you can't do

#if defined(__has_cpp_attribute) && __has_cpp_attribute(likely)

as that errors if __has_cpp_attribute is not defined

ned14 commented 1 year ago

That can be merged when the tests all pass.

Thanks for the BR and providing a fix!