This is an archive of the discontinued LLVM Phabricator instance.

Stop wrapping __has_include in another macro
ClosedPublic

Authored by arichardson on Jul 9 2018, 4:10 AM.

Details

Summary

This is not guaranteed to work since the characters after 'has_include('
have special lexing rules that can't possibly be applied when
has_include is generated by a macro. It also breaks the crash reproducers
generated by -frewrite-includes (see https://llvm.org/pr37990).

Event Timeline

arichardson created this revision.Jul 9 2018, 4:10 AM
EricWF added a subscriber: mclow.lists.

Where are the special lexing rules specified?

include/__config
153

I do prefer not hijacking this name, but if it's needed to make things work, then it's OK with me.

@mclow.lists Are you OK if we steal this identifier and #define it ourselves.

Where are the special lexing rules specified?

The C++ standard appears to be missing a rule that says that a __has_include token produced by macro expansion results in undefined behavior (matching the corresponding rule for a 'defined' token produced by macro expansion), but that seems to just be a wording oversight. You should not expect this to work.

Looks like the special rules are explained in 3.2.2 of https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations#recs.hasinc

In the first form of the has-include-expression, the parenthesized header-name token is not subject to macro expansion. The second and third forms are considered only if the first form does not match, and the preprocessing tokens are processed just as in normal text.

While there is no mention of the define to 0 if not present for __has_include it is used in the example for __has_cpp_attribute in https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations#recs.hasattr.

include/__config
153

This is the correct way of dealing with missing __has_include() according to @rsmith

rsmith added inline comments.Jul 23 2018, 5:18 PM
include/__config
153

If you would prefer to not define this name yourself, the other option would be to replace all uses of __has_include(blah) with

#ifdef __has_include
#if __has_include(blah)
//...
#endif
#endif
1062–1075

Example:

#if defined(__MINGW32__) && defined(__has_include)
#if __has_include(<pthread.h>)
#define _LIBCPP_MINGW32_HAS_PTHREAD
#endif
#endif
#if defined(__FreeBSD__) || \
// [...]
    defined(__sun__) || \
    defined(_LIBCPP_MINGW32_HAS_PTHREAD)
mclow.lists accepted this revision.Jul 23 2018, 5:27 PM
mclow.lists added inline comments.
include/__config
153

Agreed with @EricWF in general - we prefer to wrap rather than hijack other people's names.

However, we can't wrap here, so I'm ok with the hijack.

This revision is now accepted and ready to land.Jul 23 2018, 5:27 PM
This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.