[clang] Suppress warnings for tautological comparison in generated macro code

[protze.joachim]

Summary

The warnings for some of the generated and included files can quickly fill pages and might hide serious issues. The macro purposely performs tautological comparison to filter those cases. Filtering the warning for the respective code regions seems a reasonable approach.

[aaron.ballman]

This sort of diagnostic suppression is not portable and will require checking for #ifdef __clang__. Personally, I'm not fond of this style of diagnostic suppression (non-portable and rather distracting due to verbosity), but I don't know that there's a better way to do the suppression in this case...

[protze.joachim]

I don't think, this needs ifdefs. As I understand, a compiler is supposed to ignore unknown pragmas.
I fully agree that the suppression will not be compatible with other compilers.

[aaron.ballman]

In D103885#2805217, @protze.joachim wrote:

I don't think, this needs ifdefs. As I understand, a compiler is supposed to ignore unknown pragmas.

It does. You can see it produces diagnostics in -Wall mode in GCC and it produces diagnostics at any warning level in MSVC or ICC (https://godbolt.org/z/58nqGx14o). We don't use it often in the code base, but the very select places we've done it, we've protected it (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/Hexagon/HexagonDepDecoders.inc#L12, https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/StringRef.h#L878 which are the only two non-test places in the Clang + LLVM + extra code base we've done this before).

[ychen]

We could use a table to avoid the warning and speed the build a little bit. https://reviews.llvm.org/D98110

[aaron.ballman]

In D103885#2805758, @ychen wrote:

We could use a table to avoid the warning and speed the build a little bit. https://reviews.llvm.org/D98110

That's an interesting approach, thank you for sharing it! I wonder how use of a table impacts performance given that the table is on the stack and there can be a lot of options (a bit less worried for things like OpenMP kinds or overloadable operators as those as much smaller)?

[ychen]

In D103885#2805988, @aaron.ballman wrote:

In D103885#2805758, @ychen wrote:

We could use a table to avoid the warning and speed the build a little bit. https://reviews.llvm.org/D98110

That's an interesting approach, thank you for sharing it! I wonder how use of a table impacts performance given that the table is on the stack and there can be a lot of options (a bit less worried for things like OpenMP kinds or overloadable operators as those as much smaller)?

I think performance should be better if there are noticeable differences at all. The code footprint is much smaller although there are additional data loads that are most likely in the cache.

clang-tools-extra/clangd/CompileCommands.cpp

auto AddAlias = [&](DriverID Self, DriverID T) {
         PrevAlias[NextAlias[T]] = Self;
         NextAlias[Self] = NextAlias[T];
       }
       PrevAlias[Self] = T;
       NextAlias[T] = Self;
     };
     // Also grab prefixes for each option, these are not fully exposed.
     const char *const *Prefixes[DriverID::LastOption] = {nullptr};
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wtautological-compare"
+    // the generated code is expected to generate tautological comparison
 #define PREFIX(NAME, VALUE) static const char *const NAME[] = VALUE;
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
                HELP, METAVAR, VALUES)                                          \
   if (DriverID::OPT_##ALIAS != DriverID::OPT_INVALID && ALIASARGS == nullptr)  \
     AddAlias(DriverID::OPT_##ID, DriverID::OPT_##ALIAS);                       \
   Prefixes[DriverID::OPT_##ID] = PREFIX;
 #include "clang/Driver/Options.inc"
 #undef OPTION
 #undef PREFIX
+#pragma clang diagnostic pop
 
     auto Result = std::make_unique<TableTy>();
     // Iterate over distinct options (represented by the canonical alias).
     // Every spelling of this option will get the same set of rules.
     for (unsigned ID = 1 /*Skip INVALID */; ID < DriverID::LastOption; ++ID) {
       if (PrevAlias[ID] || ID == DriverID::OPT_Xclang)
         continue; // Not canonical, or specially handled.
       llvm::SmallVector<Rule> Rules;

clang/lib/Sema/SemaCodeComplete.cpp

void Sema::CodeCompleteOperatorName(Scope *S) {
 
   typedef CodeCompletionResult Result;
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
                         CodeCompleter->getCodeCompletionTUInfo(),
                         CodeCompletionContext::CCC_Type,
                         &ResultBuilder::IsType);
   Results.EnterNewScope();
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wtautological-compare"
+  // The generated code is expected to generate tautological comparison
+
   // Add the names of overloadable operators. Note that OO_Conditional is not
   // actually overloadable.
 #define OVERLOADED_OPERATOR(Name, Spelling, Token, Unary, Binary, MemberOnly)  \
   if (OO_##Name != OO_Conditional)                                             \
     Results.AddResult(Result(Spelling));
 #include "clang/Basic/OperatorKinds.def"
+#pragma clang diagnostic pop
 
   // Add any type names visible from the current scope
   Results.allowNestedNameSpecifiers();
   CodeCompletionDeclConsumer Consumer(Results, CurContext);
   LookupVisibleDecls(S, LookupOrdinaryName, Consumer,
                      CodeCompleter->includeGlobals(),
                      CodeCompleter->loadExternal());
 

llvm/lib/Frontend/OpenMP/OMPContext.cpp

OMPContext::OMPContext(bool IsDeviceCompilation, Triple TargetTriple) {
   case Triple::nvptx:
   case Triple::nvptx64:
     ActiveTraits.set(unsigned(TraitProperty::device_kind_gpu));
     break;
   default:
     break;
   }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wtautological-compare"
+  // The generated code is expected to generate tautological comparison
+
   // Add the appropriate device architecture trait based on the triple.
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str)         \
   if (TraitSelector::TraitSelectorEnum == TraitSelector::device_arch) {        \
     if (TargetTriple.getArch() == TargetTriple.getArchTypeForLLVMName(Str))    \
       ActiveTraits.set(unsigned(TraitProperty::Enum));                         \
     if (StringRef(Str) == StringRef("x86_64") &&                               \
         TargetTriple.getArch() == Triple::x86_64)                              \
       ActiveTraits.set(unsigned(TraitProperty::Enum));                         \
   }
 #include "llvm/Frontend/OpenMP/OMPKinds.def"
+#pragma clang diagnostic pop
 
   // TODO: What exactly do we want to see as device ISA trait?
   //       The discussion on the list did not seem to have come to an agreed
   //       upon solution.
 
   // LLVM is the "OpenMP vendor" but we could also interpret vendor as the
   // target vendor.
   ActiveTraits.set(unsigned(TraitProperty::implementation_vendor_llvm));