Fix isInSystemMacro to handle pasted token

[serge-sans-paille]

Summary

Token pasted by the preprocessor (through ##) have a Spelling pointing to scratch buffer. As a result they are not recognized at system macro, even though the pasting happened in a system macro. Fix that by looking into the parent macro if the original lookup is not enough.

This effectively fixes https://bugs.llvm.org/show_bug.cgi?id=35268, but I'm not quite sure about the kind of test case I should add with this patch?

[serge-sans-paille]

Test case added

[serge-sans-paille]

@rsmith up!

[rsmith]

include/clang/Basic/SourceManager.h

1458–1460 ↗

(On Diff #181498)

I think this also matches things like:

// system header
#define BAR FOO

// user code
#define FOO x
BAR

... where the location of x token produced by the BAR macro will now be considered to be in a system macro. Can you make this more targeted? (Maybe we could change the SLocEntry for the token-paste scratch buffer to be considered a system header if the ## token is in a system macro -- that'll need the preprocessor to store two ScratchBuffers, one for system headers and one for user code, but that seems acceptable.)

[serge-sans-paille]

Patch updated to take into account @rsmith remarks. Test case added as an illustration.

[serge-sans-paille]

include/clang/Basic/SourceManager.h

1458–1460 ↗

(On Diff #181498)

Thanks for the advice! This approach should be more efficient and avoid the situation you point out - I've added a test case to illustrate that.

[rsmith]

include/clang/Basic/SourceManager.h

1466 ↗

(On Diff #184275)

Please add a space after if.

[serge-sans-paille]

It's in, thanks @rsmith for the review.

cfe/trunk/include/clang/Basic/SourceManager.h

public:
   }
 
   /// Returns whether \p Loc is located in a <command line> file.
   bool isWrittenInCommandLineFile(SourceLocation Loc) const {
     StringRef Filename(getPresumedLoc(Loc).getFilename());
     return Filename.equals("<command line>");
   }
 
+  /// Returns whether \p Loc is located in a <scratch space> file.
+  bool isWrittenInScratchSpace(SourceLocation Loc) const {
+    StringRef Filename(getPresumedLoc(Loc).getFilename());
+    return Filename.equals("<scratch space>");
+  }
+
   /// Returns if a SourceLocation is in a system header.
   bool isInSystemHeader(SourceLocation Loc) const {
     return isSystem(getFileCharacteristic(Loc));
   }
 
   /// Returns if a SourceLocation is in an "extern C" system header.
   bool isInExternCSystemHeader(SourceLocation Loc) const {
     return getFileCharacteristic(Loc) == SrcMgr::C_ExternCSystem;
   }
 
   /// Returns whether \p Loc is expanded from a macro in a system header.
   bool isInSystemMacro(SourceLocation loc) const {
-    return loc.isMacroID() && isInSystemHeader(getSpellingLoc(loc));
+    if(!loc.isMacroID())
+      return false;
+
+    // This happens when the macro is the result of a paste, in that case
+    // its spelling is the scratch memory, so we take the parent context.
+    if (isWrittenInScratchSpace(getSpellingLoc(loc))) {
+      return isInSystemHeader(getSpellingLoc(getImmediateMacroCallerLoc(loc)));
+    }
+    else {
+      return isInSystemHeader(getSpellingLoc(loc));
+    }
   }
 
   /// The size of the SLocEntry that \p FID represents.
   unsigned getFileIDSize(FileID FID) const;
 
   /// Given a specific FileID, returns true if \p Loc is inside that
   /// FileID chunk and sets relative offset (offset of \p Loc from beginning
   /// of FileID) to \p relativeOffset.

cfe/trunk/test/Misc/no-warn-in-system-macro.c

+// RUN: %clang_cc1 -isystem %S -Wdouble-promotion -fsyntax-only %s  2>&1 | FileCheck -allow-empty %s
+// CHECK-NOT: warning:
+
+#include <no-warn-in-system-macro.c.inc>
+
+int main(void)
+{
+  double foo = 1.0;
+
+  if (isnan(foo))
+    return 1;
+  return 0;
+}

cfe/trunk/test/Misc/no-warn-in-system-macro.c.inc

+extern int __isnanf(float f);
+extern int __isnan(double f);
+extern int __isnanl(long double f);
+#define isnan(x) \
+  (sizeof (x) == sizeof (float)                \
+  ? __isnanf (x)                    \
+  : sizeof (x) == sizeof (double)               \
+  ? __isnan (x) : __isnanl (x))
+

cfe/trunk/test/Misc/warn-in-system-macro-def.c

+// RUN: %clang_cc1 -isystem %S -Wdouble-promotion -fsyntax-only %s  2>&1 | FileCheck -allow-empty %s
+// CHECK: warning:
+// CHECK: expanded from macro 'ISNAN'
+// CHECK: expanded from macro 'isnan'
+
+#include <warn-in-system-macro-def.c.inc>
+
+#define isnan(x) \
+  (sizeof (x) == sizeof (float)                \
+  ? __isnanf (x)                    \
+  : sizeof (x) == sizeof (double)               \
+  ? __isnan (x) : __isnanl (x))
+
+int main(void)
+{
+  double foo = 1.0;
+
+  if (ISNAN(foo))
+    return 1;
+  return 0;
+}

cfe/trunk/test/Misc/warn-in-system-macro-def.c.inc

+extern int __isnanf(float f);
+extern int __isnan(double f);
+extern int __isnanl(long double f);
+#define ISNAN isnan