It turns out that clang's isBeforeInTranslationUnitThan

doesn't distinguish between symbols in the same macro (or if
it does, then not in a way that I'm able to take advantage of,
perhaps because I'm comparing a decl to a SourceLocation
rather than two decls).  Special-case that situation to just
always say "yes, a is before b".  This fixes the tests (which
I've also augmented to capture outside badinc.h).

Submitting TBR because it's the weekend and I'd like the build
to be green again.

R=wan
DELTA=33  (32 added, 0 deleted, 1 changed)


Revision created by MOE tool push_codebase.
MOE_MIGRATION=1599

iwyu_location_util.h

@@ -204,6 +204,19 @@ template<typename T, typename U>
 inline bool IsBeforeInTranslationUnit(const T& a, const U& b) {
   const clang::FullSourceLoc a_loc(GetLocation(a), *GlobalSourceManager());
   const clang::FullSourceLoc b_loc(GetLocation(b), *GlobalSourceManager());
+  // Inside a macro, everything has the same instantiation location.
+  // We'd like to use spelling-location to break that tie, but it's
+  // unreliable since a or b might be spelled in "<scratch space>".
+  // So we're just conservative and return true always if the two have
+  // an equal location and are in a macro.  (Because we check the
+  // instantiation-location is equal, it's enough that one of the two
+  // be in a macro; we prefer that since IsInMacro fails if T or U is
+  // the wrong type.)  TODO(csilvers): see if's possible to get
+  // isBeforeInTranslationUnitThan working properly.  This may require
+  // storing source-locations better in OneUse.
+  if ((IsInMacro(a_loc) || IsInMacro(b_loc)) &&
+      GetInstantiationLoc(a_loc) == GetInstantiationLoc(b_loc))
+    return true;
   return a_loc.isBeforeInTranslationUnitThan(b_loc);
 }
 

tests/macro_location-d2.h

@@ -17,6 +17,19 @@
     OtherClass o;                               \
   };
 
+// This macro is tricky because myclass_##classname involves a type
+// that's defined in scratch space.  Make sure this doesn't result in
+// an IWYU violation.  Nor should classname used *not* in a macro
+// concatenation (as the return value of Init).
+#define USE_CLASS(classname)                    \
+  struct Use_##classname {                      \
+    Use_##classname() { Init(); }               \
+    classname* Init() { return 0; }             \
+  };                                            \
+  static Use_##classname myclass_##classname
+
+#define CREATE_VAR(typ)   typ create_var
+
 
 /**** IWYU_SUMMARY
 

tests/macro_location.h

@@ -13,10 +13,16 @@
 class Foo;
 static Foo *foo = 0;
 
+class HClass { };
+
 // IWYU: Foo is...*macro_location-i3.h
 const int s = ARRAYSIZE(foo);
 // Should not need a declaration of NewClass_Bar, or NewClass.
 NEW_CLASS(Bar);
+// This shouldn't cause weird iwyu issues between us and macro_location-d2.h.
+USE_CLASS(HClass);
+// This shouldn't cause macro_location-d1.h to need to include us for HClass.
+CREATE_VAR(HClass);
 
 
 /**** IWYU_SUMMARY
@@ -29,7 +35,7 @@ tests/macro_location.h should remove these lines:
 - class Foo;  // lines XX-XX
 
 The full include-list for tests/macro_location.h:
-#include "tests/macro_location-d2.h"  // for ARRAYSIZE, NEW_CLASS
+#include "tests/macro_location-d2.h"  // for ARRAYSIZE, CREATE_VAR, NEW_CLASS, USE_CLASS
 #include "tests/macro_location-i3.h"  // for Foo