Suppress -pedantic warnings about GNU extension StmtExpr in glibc's assert macro

[sberg]

Summary

See the mail thread starting at http://lists.llvm.org/pipermail/cfe-dev/2017-July/054666.html "[cfe-dev] -pedantic warnings in system headers?":

The assert.h in glibc 2.25 defines assert as a GNU-extension "statement expression", see https://sourceware.org/git/?p=glibc.git;a=commit;h=e077349ce589466eecd47213db4fae6b80ec18c4 "assert.h: allow gcc to detect assert(a = 1) errors". That caused Clang -pedantic to emit -Wgnu-statement-expression warnings (unlike GCC).

This patch extends the heuristic when to suppress warnings in system headers, to also suppress warnings about extensions used in the body of a function-like macro defined in a system header. (The "function-like" part is there so that the warning from the body of LONG_LONG_MAX in clang/test/CXX/drs/dr4xx.cpp is still emitted.)

I ran into this when trying to build CoinMP (which wants to be built with -pedantic-errors) as part of building LibreOffice master with Clang trunk against Fedora 26 glibc. I'm not sure what the best solution is, though: Either extend the heuristic when to suppress warnings as done here (which has the benefit of aligning the behavior with GCC), or change the CoinMP build system to not insist on -pedantic-error.

[dblaikie]

Ping - Richard, what'd you reckon? (This came up on another review where -Wzero-as-null-pointer triggered in a system header in libstdc++ I think)

[raj.khem]

There is a proposed fix for glibc's assert here

https://sourceware.org/ml/libc-alpha/2017-07/msg00227.html

despite that, this patch in general seems useful.

[sberg]

(the original diff had inadvertently lacked full -U999999 context; uploaded a new diff)

clang/lib/Basic/DiagnosticIDs.cpp

if (DiagID >= diag::DIAG_UPPER_LIMIT) {
     return CustomDiagInfo->getLevel(DiagID);
   }
 
   unsigned DiagClass = getBuiltinDiagClass(DiagID);
   if (DiagClass == CLASS_NOTE) return DiagnosticIDs::Note;
   return toLevel(getDiagnosticSeverity(DiagID, Loc, Diag));
 }
 
+static bool isSystemFunctionMacroBodyExpansion(SourceLocation Loc,
+                                               const SourceManager &SM) {
+  if (!(SM.isInSystemMacro(Loc) && SM.isMacroBodyExpansion(Loc)))
+    return false;
+  auto Range = SM.getImmediateExpansionRange(Loc);
+  return Range.first != Range.second;
+}
+
 /// \brief Based on the way the client configured the Diagnostic
 /// object, classify the specified diagnostic ID into a Level, consumable by
 /// the DiagnosticClient.
 ///
 /// \param Loc The source location we are interested in finding out the
 /// diagnostic state. Can be null in order to query the latest state.
 diag::Severity
 DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc,
DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc,
   // Custom diagnostics always are emitted in system headers.
   bool ShowInSystemHeader =
       !GetDiagInfo(DiagID) || GetDiagInfo(DiagID)->WarnShowInSystemHeader;
 
   // If we are in a system header, we ignore it. We look at the diagnostic class
   // because we also want to ignore extensions and warnings in -Werror and
   // -pedantic-errors modes, which *map* warnings/extensions to errors.
   if (State->SuppressSystemWarnings && !ShowInSystemHeader && Loc.isValid() &&
-      Diag.getSourceManager().isInSystemHeader(
-          Diag.getSourceManager().getExpansionLoc(Loc)))
+      (Diag.getSourceManager().isInSystemHeader(
+          Diag.getSourceManager().getExpansionLoc(Loc)) ||
+       (IsExtensionDiag &&
+        isSystemFunctionMacroBodyExpansion(Loc, Diag.getSourceManager()))))
     return diag::Severity::Ignored;
 
   return Result;
 }
 
 #define GET_DIAG_ARRAYS
 #include "clang/Basic/DiagnosticGroups.inc"
 #undef GET_DIAG_ARRAYS

clang/test/SemaCXX/warn-sysheader-macro.cpp

-// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow -Wold-style-cast %s
+// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow -Wold-style-cast -pedantic %s
 
 // Test that macro expansions from system headers don't trigger 'syntactic'
 // warnings that are not actionable.
 
 #ifdef IS_SYSHEADER
 #pragma clang system_header
 
 #define SANITY(a) (a / 0)
 
 #define SHADOW(a) __extension__({ int v = a; v; })
 
 #define OLD_STYLE_CAST(a) ((int) (a))
 
+#define ASSERT(a) ({ if (a) {} else {} })
+
 #else
 
 #define IS_SYSHEADER
 #include __FILE__
 
 void testSanity() {
   // Validate that the test is set up correctly
   int i = SANITY(0); // expected-warning {{division by zero is undefined}}
 }
 
 void PR16093() {
   // no -Wshadow in system macro expansion
   int i = SHADOW(SHADOW(1));
 }
 
 void PR18147() {
   // no -Wold_style_cast in system macro expansion
   int i = OLD_STYLE_CAST(0);
 }
 
+void testExtension() {
+  ASSERT(true);
+  ASSERT(({ true; })); // expected-warning {{use of GNU statement expression extension}}
+}
+
 #endif