This is an archive of the discontinued LLVM Phabricator instance.

Add a check for GCC to the _LIBCPP_EXPLICIT define
ClosedPublic

Authored by mdaniels on Oct 28 2016, 6:04 PM.

Details

Summary

When using libc++ with GCC functions are not getting the explicit specifier as expected. This change adds a GCC check to resolve the problem.

I was unable to figure out exactly when GCC added support for the explicit keyword, but I found messages on the mailing list suggesting it was implemented and working in at lease GCC 3.3, so it just defaults to any GCC version.

A test case is included to reproduce the problem. While it could be demonstrated with any explicit function, this just happened to be the one we were having problem with.

Tested with GCC 5.4.0 and clang 3.8.0 using libc++ from ToT as well as GCC 5.4 using libc++ 3.7.1.

Diff Detail

Event Timeline

mdaniels updated this revision to Diff 76280.Oct 28 2016, 6:04 PM
mdaniels retitled this revision from to Add a check for GCC to the _LIBCPP_EXPLICIT define.
mdaniels updated this object.
mdaniels updated this revision to Diff 76285.Oct 28 2016, 6:07 PM
mdaniels updated this object.

Did some more digging to find the right version check..

mclow.lists added inline comments.Nov 1 2016, 8:11 PM
test/std/diagnostics/syserr/syserr.errcode/syserr.errcode.observers/bool.fail.cpp
19

I think this test would be better making the bool conversion clear; instead of relying on the double conversion error_code --> bool --> int.

bool whatever()
{
    const std::error_code ec(0, std::generic_category());
    return ec;
}

int main () {}
mdaniels updated this revision to Diff 76698.Nov 2 2016, 7:09 AM

Update based on feedback.

mclow.lists accepted this revision.Nov 2 2016, 7:13 AM
mclow.lists edited edge metadata.

LGTM

This revision is now accepted and ready to land.Nov 2 2016, 7:13 AM
mdaniels edited edge metadata.Nov 2 2016, 9:33 AM
mdaniels added a subscriber: cfe-commits.

I don't have commit access, could I trouble someone to do it on my behalf?

Ok, this is weird. It looks like the changes to <__config> got committed, but not the test.

EricWF edited edge metadata.Jan 23 2017, 12:55 PM

I think I may have committed these changes as part of another change set.

mclow.lists closed this revision.Jan 23 2019, 3:10 PM

Yes, this appears to have been landed. Closing.