Page MenuHomePhabricator

Warn about usage of __has_include/__has_include_next in macro expansions
Needs ReviewPublic

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

Details

Reviewers
rsmith
Summary

The characters after __has_include( have special lexing rules that can't
possibly be applied when __has_include is generated by a macro. Instead of
wrapping __has_include in another macro the following should be used instead:

#ifndef __has_include
#define __has_include(...) 0
#endif

This warning should fix the underlying issue for https://llvm.org/pr37990

Diff Detail

Event Timeline

arichardson created this revision.Jul 9 2018, 10:52 AM

Ping? I'm not sure if this is still required now that D63508 has been committed?

Herald added a project: Restricted Project. · View Herald TranscriptSep 18 2019, 2:21 PM

Under http://eel.is/c++draft/cpp#cond-7.sentence-2, the identifier __has_include can't appear anywhere other than in the context of an #if, #elif, #iifdef, or #ifndef. That's what we should be checking for and diagnosing here (and we should produce an ExtWarn rather than a Warning for this case, because such code is ill-formed, and accepting it at all is a language extension compared to the C++ rules). We should apply the same behavior to __has_cpp_attribute, to which the same rule applies.

I'm picking this up on the Qt side and removing our wrapper macro. Does this apply to other cases as well? The standard mentions _­_­has_­cpp_­attribute, but what about __has_builtin, __has_attribute, and __has_include_next?

I'm picking this up on the Qt side and removing our wrapper macro. Does this apply to other cases as well? The standard mentions _­_­has_­cpp_­attribute, but what about __has_builtin, __has_attribute, and __has_include_next?

The standard can only talk about __has_cpp_attribute and __has_include because the others are extensions. We should treat __has_cpp_attribute and __has_c_attribute the same way, and both should be handled the same as __has_include in terms of http://eel.is/c++draft/cpp#cond-7.sentence-2.

I see no reason to treat __has_attribute or __has_include_next differently from __has_cpp_attribute or __has_include. I am on the fence about __has_builtin because I could sort of imagine wanting to vary runtime behavior in a case like this:

if (__has_builtin(__builtin_whatever)) {
  return __builtin_whatever();
} else {
  // Do it the hard way
}

(The same is not true for things like __has_cpp_attribute or __has_include.)

jfb edited the summary of this revision. (Show Details)Dec 9 2019, 10:56 AM

https://codereview.qt-project.org/c/qt/qtbase/+/284037 has merged and will be part of Qt 5.14.1

I went with treating them all under one strategy:

#ifndef __has_foo
#define __has_foo(x) 0
#endif

This includes __has_builtin, __has_feature, __has_attribute, __has_cpp_attribute, __has_include, and __has_include_next.