Warn about usage of __has_include/__has_include_next in macro expansions

[arichardson]

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

[arichardson]

Ping?

[arichardson]

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

[rsmith]

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.

[torarnv]

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?

[torarnv]

Qt fix in https://codereview.qt-project.org/c/qt/qtbase/+/284037

[aaron.ballman]

In D49091#1773190, @torarnv wrote:

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.)

[torarnv]

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.

include/clang/Basic/DiagnosticLexKinds.td

 
 def warn_defined_in_object_type_macro : Warning<
   "macro expansion producing 'defined' has undefined behavior">,
   InGroup<ExpansionToDefined>;
 def warn_defined_in_function_type_macro : Extension<
   "macro expansion producing 'defined' has undefined behavior">,
   InGroup<ExpansionToDefined>;
 
+def warn_has_include_in_macro_expansion : Warning<
+  "macro expansion producing %0 has undefined behavior">,
+InGroup<DiagGroup<"expansion-to-has-include">>;
+
+
 let CategoryName = "Nullability Issue" in {
 
 def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">;
 def err_pp_double_begin_of_assume_nonnull : Error<
   "already inside '#pragma clang assume_nonnull'">;
 def err_pp_unmatched_end_of_assume_nonnull : Error<
   "not currently inside '#pragma clang assume_nonnull'">;
 def err_pp_include_in_assume_nonnull : Error<
   "cannot #include files inside '#pragma clang assume_nonnull'">;
 def err_pp_eof_in_assume_nonnull : Error<
   "'#pragma clang assume_nonnull' was not ended within this file">;
 
 }
 
 }

lib/Lex/PPMacroExpansion.cpp

if (PP.getCurrentLexer()) {
       PP.getCurrentLexer()->LexIncludeFilename(Tok);
     } else {
       // We're in a macro, so we can't use LexIncludeFilename; just
       // grab the next token.
       PP.Lex(Tok);
     }
   }
 
+  if (LParenLoc.isMacroID())
+    // The characters after '__has_include(' have special lexing rules that
+    // can't possibly be applied when __has_include is generated by a macro.
+    // Diagnose this to avoid problems such as https://llvm.org/pr37990
+    PP.Diag(LParenLoc, diag::warn_has_include_in_macro_expansion) << II;
+
   // Reserve a buffer to get the spelling.
   SmallString<128> FilenameBuffer;
   StringRef Filename;
   SourceLocation EndLoc;
   
   switch (Tok.getKind()) {
   case tok::eod:
     // If the token kind is EOD, the error has already been diagnosed.

test/Preprocessor/expr_has_include_expansion.c

+// RUN: %clang_cc1 %s -E -verify -o /dev/null
+
+#define __libcpp_has_include(x) __has_include(x)
+#if __libcpp_has_include(<chrono>) // expected-warning{{macro expansion producing '__has_include' has undefined behavior}}
+#endif
+
+#define QT_HAS_INCLUDE_NEXT(x) __has_include_next(x)
+#if QT_HAS_INCLUDE_NEXT(<sys/foo.h>) // expected-warning{{macro expansion producing '__has_include_next' has undefined behavior}} expected-warning{{#include_next in primary source file}}
+#endif
+
+#define HAS_CHRONO __has_include(<chrono>)
+#if HAS_CHRONO // expected-warning{{macro expansion producing '__has_include' has undefined behavior}}
+#endif
+
+#define HAS_QUOTED_NEXT __has_include_next("foo.h")
+#if HAS_QUOTED_NEXT // expected-warning{{macro expansion producing '__has_include_next' has undefined behavior}} expected-warning{{#include_next in primary source file}}
+#endif
+
+// check that this diagnostic doesn't warn on normal __has_include() usage
+#if __has_include("foo.h") || __has_include_next("bar.h") // expected-warning{{#include_next in primary source file}}
+#endif