[Modules] Teach Clang to survive ambiguous macros which come from system
authorChandler Carruth <chandlerc@gmail.com>
Fri, 13 Mar 2015 08:29:54 +0000 (08:29 +0000)
committerChandler Carruth <chandlerc@gmail.com>
Fri, 13 Mar 2015 08:29:54 +0000 (08:29 +0000)
headers even if they arrived when merging non-system modules.

The idea of this code is that we don't want to warn the user about
macros defined multiple times by their system headers with slightly
different definitions. We should have this behavior if either the
macro comes from a system module, or the definition within the module
comes from a system header. Previously, we would warn on ambiguous
macros being merged when they came from a users modules even though they
only showed up via system headers.

By surviving this we can handle common system header macro differences
like differing 'const' qualification of pointers due to some headers
predating 'const' being valid in C code, even when those systems headers
are pre-built into a system module.

Differential Revision: http://reviews.llvm.org/D8310

llvm-svn: 232149

13 files changed:
clang/lib/Serialization/ASTReader.cpp
clang/test/Modules/Inputs/macro-ambiguity/a/quote/a_quote.h [new file with mode: 0644]
clang/test/Modules/Inputs/macro-ambiguity/a/system/a_system.h [new file with mode: 0644]
clang/test/Modules/Inputs/macro-ambiguity/b/quote/b_quote.h [new file with mode: 0644]
clang/test/Modules/Inputs/macro-ambiguity/b/system/b_system.h [new file with mode: 0644]
clang/test/Modules/Inputs/macro-ambiguity/c/quote/c_quote.h [new file with mode: 0644]
clang/test/Modules/Inputs/macro-ambiguity/c/system/c_system.h [new file with mode: 0644]
clang/test/Modules/Inputs/macro-ambiguity/d/quote/d_quote.h [new file with mode: 0644]
clang/test/Modules/Inputs/macro-ambiguity/d/system/d_system.h [new file with mode: 0644]
clang/test/Modules/Inputs/macro-ambiguity/e/quote/e_quote.h [new file with mode: 0644]
clang/test/Modules/Inputs/macro-ambiguity/e/system/e_system.h [new file with mode: 0644]
clang/test/Modules/Inputs/macro-ambiguity/module.modulemap [new file with mode: 0644]
clang/test/Modules/macro-ambiguity.cpp [new file with mode: 0644]

index 2ee4975..2e4e891 100644 (file)
@@ -1950,15 +1950,13 @@ static bool areDefinedInSystemModules(MacroInfo *PrevMI, MacroInfo *NewMI,
   Module *PrevOwner = nullptr;
   if (SubmoduleID PrevModID = PrevMI->getOwningModuleID())
     PrevOwner = Reader.getSubmodule(PrevModID);
-  SourceManager &SrcMgr = Reader.getSourceManager();
-  bool PrevInSystem
-    = PrevOwner? PrevOwner->IsSystem
-               : SrcMgr.isInSystemHeader(PrevMI->getDefinitionLoc());
-  bool NewInSystem
-    = NewOwner? NewOwner->IsSystem
-              : SrcMgr.isInSystemHeader(NewMI->getDefinitionLoc());
   if (PrevOwner && PrevOwner == NewOwner)
     return false;
+  SourceManager &SrcMgr = Reader.getSourceManager();
+  bool PrevInSystem = (PrevOwner && PrevOwner->IsSystem) ||
+                      SrcMgr.isInSystemHeader(PrevMI->getDefinitionLoc());
+  bool NewInSystem = (NewOwner && NewOwner->IsSystem) ||
+                     SrcMgr.isInSystemHeader(NewMI->getDefinitionLoc());
   return PrevInSystem && NewInSystem;
 }
 
diff --git a/clang/test/Modules/Inputs/macro-ambiguity/a/quote/a_quote.h b/clang/test/Modules/Inputs/macro-ambiguity/a/quote/a_quote.h
new file mode 100644 (file)
index 0000000..efe6fa1
--- /dev/null
@@ -0,0 +1,8 @@
+#ifndef A_QUOTE_H
+#define A_QUOTE_H
+
+#define FOO1_QUOTE(x) x + x
+#define BAR1_QUOTE(x) x + x
+#define BAZ1_QUOTE(x) x + x
+
+#endif
diff --git a/clang/test/Modules/Inputs/macro-ambiguity/a/system/a_system.h b/clang/test/Modules/Inputs/macro-ambiguity/a/system/a_system.h
new file mode 100644 (file)
index 0000000..f9f3287
--- /dev/null
@@ -0,0 +1,15 @@
+#ifndef A_SYSTEM_H
+#define A_SYSTEM_H
+
+// FIXME: We have to use this to mark the header as a system header in
+// a module because header search didn't actually occur and so we can't have
+// found the header via system header search, even though when we map to this
+// header and load the module we will have mapped to the header by finding it
+// via system header search.
+#pragma GCC system_header
+
+#define FOO1_SYSTEM(x) x + x
+#define BAR1_SYSTEM(x) x + x
+#define BAZ1_SYSTEM(x) x + x
+
+#endif
diff --git a/clang/test/Modules/Inputs/macro-ambiguity/b/quote/b_quote.h b/clang/test/Modules/Inputs/macro-ambiguity/b/quote/b_quote.h
new file mode 100644 (file)
index 0000000..f5e990f
--- /dev/null
@@ -0,0 +1,8 @@
+#ifndef B_QUOTE_H
+#define B_QUOTE_H
+
+#define FOO2_QUOTE(x) x + x
+#define BAR2_QUOTE(x) x + x
+#define BAZ2_QUOTE(x) x + x
+
+#endif
diff --git a/clang/test/Modules/Inputs/macro-ambiguity/b/system/b_system.h b/clang/test/Modules/Inputs/macro-ambiguity/b/system/b_system.h
new file mode 100644 (file)
index 0000000..5b55303
--- /dev/null
@@ -0,0 +1,15 @@
+#ifndef B_SYSTEM_H
+#define B_SYSTEM_H
+
+// FIXME: We have to use this to mark the header as a system header in
+// a module because header search didn't actually occur and so we can't have
+// found the header via system header search, even though when we map to this
+// header and load the module we will have mapped to the header by finding it
+// via system header search.
+#pragma GCC system_header
+
+#define FOO2_SYSTEM(x) x + x
+#define BAR2_SYSTEM(x) x + x
+#define BAZ2_SYSTEM(x) x + x
+
+#endif
diff --git a/clang/test/Modules/Inputs/macro-ambiguity/c/quote/c_quote.h b/clang/test/Modules/Inputs/macro-ambiguity/c/quote/c_quote.h
new file mode 100644 (file)
index 0000000..1314300
--- /dev/null
@@ -0,0 +1,7 @@
+#ifndef C_QUOTE_H
+#define C_QUOTE_H
+
+#define FOO1_QUOTE(x) 2 * x
+#define FOO2_QUOTE(x) 2 * x
+
+#endif
diff --git a/clang/test/Modules/Inputs/macro-ambiguity/c/system/c_system.h b/clang/test/Modules/Inputs/macro-ambiguity/c/system/c_system.h
new file mode 100644 (file)
index 0000000..25e795d
--- /dev/null
@@ -0,0 +1,14 @@
+#ifndef C_SYSTEM_H
+#define C_SYSTEM_H
+
+// FIXME: We have to use this to mark the header as a system header in
+// a module because header search didn't actually occur and so we can't have
+// found the header via system header search, even though when we map to this
+// header and load the module we will have mapped to the header by finding it
+// via system header search.
+#pragma GCC system_header
+
+#define FOO1_SYSTEM(x) 2 * x
+#define FOO2_SYSTEM(x) 2 * x
+
+#endif
diff --git a/clang/test/Modules/Inputs/macro-ambiguity/d/quote/d_quote.h b/clang/test/Modules/Inputs/macro-ambiguity/d/quote/d_quote.h
new file mode 100644 (file)
index 0000000..ac9d43b
--- /dev/null
@@ -0,0 +1,7 @@
+#ifndef D_QUOTE_H
+#define D_QUOTE_H
+
+#define BAR1_QUOTE(x) 2 * x
+#define BAR2_QUOTE(x) 2 * x
+
+#endif
diff --git a/clang/test/Modules/Inputs/macro-ambiguity/d/system/d_system.h b/clang/test/Modules/Inputs/macro-ambiguity/d/system/d_system.h
new file mode 100644 (file)
index 0000000..5c10cc1
--- /dev/null
@@ -0,0 +1,14 @@
+#ifndef D_SYSTEM_H
+#define D_SYSTEM_H
+
+// FIXME: We have to use this to mark the header as a system header in
+// a module because header search didn't actually occur and so we can't have
+// found the header via system header search, even though when we map to this
+// header and load the module we will have mapped to the header by finding it
+// via system header search.
+#pragma GCC system_header
+
+#define BAR1_SYSTEM(x) 2 * x
+#define BAR2_SYSTEM(x) 2 * x
+
+#endif
diff --git a/clang/test/Modules/Inputs/macro-ambiguity/e/quote/e_quote.h b/clang/test/Modules/Inputs/macro-ambiguity/e/quote/e_quote.h
new file mode 100644 (file)
index 0000000..7849429
--- /dev/null
@@ -0,0 +1,7 @@
+#ifndef E_QUOTE_H
+#define E_QUOTE_H
+
+#define BAZ1_QUOTE(x) 2 * x
+#define BAZ2_QUOTE(x) 2 * x
+
+#endif
diff --git a/clang/test/Modules/Inputs/macro-ambiguity/e/system/e_system.h b/clang/test/Modules/Inputs/macro-ambiguity/e/system/e_system.h
new file mode 100644 (file)
index 0000000..c9e1341
--- /dev/null
@@ -0,0 +1,7 @@
+#ifndef E_SYSTEM_H
+#define E_SYSTEM_H
+
+#define BAZ1_SYSTEM(x) 2 * x
+#define BAZ2_SYSTEM(x) 2 * x
+
+#endif
diff --git a/clang/test/Modules/Inputs/macro-ambiguity/module.modulemap b/clang/test/Modules/Inputs/macro-ambiguity/module.modulemap
new file mode 100644 (file)
index 0000000..23da294
--- /dev/null
@@ -0,0 +1,25 @@
+module a {
+  header "Inputs/macro-ambiguity/a/quote/a_quote.h"
+  header "Inputs/macro-ambiguity/a/system/a_system.h"
+  export *
+}
+module b [system] {
+  header "Inputs/macro-ambiguity/b/quote/b_quote.h"
+  header "Inputs/macro-ambiguity/b/system/b_system.h"
+  export *
+}
+module c {
+  header "Inputs/macro-ambiguity/c/quote/c_quote.h"
+  header "Inputs/macro-ambiguity/c/system/c_system.h"
+  export *
+}
+module d [system] {
+  header "Inputs/macro-ambiguity/d/quote/d_quote.h"
+  header "Inputs/macro-ambiguity/d/system/d_system.h"
+  export *
+}
+module e {
+  textual header "Inputs/macro-ambiguity/e/quote/e_quote.h"
+  textual header "Inputs/macro-ambiguity/e/system/e_system.h"
+  export *
+}
diff --git a/clang/test/Modules/macro-ambiguity.cpp b/clang/test/Modules/macro-ambiguity.cpp
new file mode 100644 (file)
index 0000000..ea9e4f5
--- /dev/null
@@ -0,0 +1,115 @@
+// RUN: rm -rf %t
+// RUN: cd %S
+//
+// RUN: %clang_cc1 -fmodules -x c++ -fmodules-cache-path=%t \
+// RUN:   -v \
+// RUN:   -iquote Inputs/macro-ambiguity/a/quote \
+// RUN:   -isystem Inputs/macro-ambiguity/a/system \
+// RUN:   -fno-implicit-modules -fno-modules-implicit-maps \
+// RUN:   -fmodule-map-file-home-is-cwd \
+// RUN:   -emit-module -fmodule-name=a -o %t/a.pcm \
+// RUN:   Inputs/macro-ambiguity/module.modulemap
+//
+// RUN: %clang_cc1 -fmodules -x c++ -fmodules-cache-path=%t \
+// RUN:   -v \
+// RUN:   -iquote Inputs/macro-ambiguity/b/quote \
+// RUN:   -isystem Inputs/macro-ambiguity/b/system \
+// RUN:   -fno-implicit-modules -fno-modules-implicit-maps \
+// RUN:   -fmodule-map-file-home-is-cwd \
+// RUN:   -emit-module -fmodule-name=b -o %t/b.pcm \
+// RUN:   Inputs/macro-ambiguity/module.modulemap
+//
+// RUN: %clang_cc1 -fmodules -x c++ -fmodules-cache-path=%t \
+// RUN:   -v \
+// RUN:   -iquote Inputs/macro-ambiguity/c/quote \
+// RUN:   -isystem Inputs/macro-ambiguity/c/system \
+// RUN:   -fno-implicit-modules -fno-modules-implicit-maps \
+// RUN:   -fmodule-map-file-home-is-cwd \
+// RUN:   -emit-module -fmodule-name=c -o %t/c.pcm \
+// RUN:   Inputs/macro-ambiguity/module.modulemap
+//
+// RUN: %clang_cc1 -fmodules -x c++ -fmodules-cache-path=%t \
+// RUN:   -v \
+// RUN:   -iquote Inputs/macro-ambiguity/d/quote \
+// RUN:   -isystem Inputs/macro-ambiguity/d/system \
+// RUN:   -fno-implicit-modules -fno-modules-implicit-maps \
+// RUN:   -fmodule-map-file-home-is-cwd \
+// RUN:   -emit-module -fmodule-name=d -o %t/d.pcm \
+// RUN:   Inputs/macro-ambiguity/module.modulemap
+//
+// RUN: %clang_cc1 -fmodules -x c++ -fmodules-cache-path=%t \
+// RUN:   -v \
+// RUN:   -iquote Inputs/macro-ambiguity/a/quote \
+// RUN:   -isystem Inputs/macro-ambiguity/a/system \
+// RUN:   -iquote Inputs/macro-ambiguity/b/quote \
+// RUN:   -isystem Inputs/macro-ambiguity/b/system \
+// RUN:   -iquote Inputs/macro-ambiguity/c/quote \
+// RUN:   -isystem Inputs/macro-ambiguity/c/system \
+// RUN:   -iquote Inputs/macro-ambiguity/d/quote \
+// RUN:   -isystem Inputs/macro-ambiguity/d/system \
+// RUN:   -iquote Inputs/macro-ambiguity/e/quote \
+// RUN:   -isystem Inputs/macro-ambiguity/e/system \
+// RUN:   -fno-implicit-modules -fno-modules-implicit-maps \
+// RUN:   -fmodule-map-file-home-is-cwd \
+// RUN:   -fmodule-map-file=Inputs/macro-ambiguity/module.modulemap \
+// RUN:   -fmodule-file=%t/a.pcm \
+// RUN:   -fmodule-file=%t/b.pcm \
+// RUN:   -fmodule-file=%t/c.pcm \
+// RUN:   -fmodule-file=%t/d.pcm \
+// RUN:   -Wambiguous-macro -verify macro-ambiguity.cpp
+
+// Include the textual headers first to maximize the ways in which things can
+// become ambiguous.
+#include "e_quote.h"
+#include <e_system.h>
+
+#include "a_quote.h"
+#include <a_system.h>
+#include "b_quote.h"
+#include <b_system.h>
+#include "c_quote.h"
+#include <c_system.h>
+#include "d_quote.h"
+#include <d_system.h>
+
+int test(int x) {
+  // We expect to get warnings for all of the quoted includes but none of the
+  // system includes here because the first module is a non-system module and
+  // the quote macros come from non-system-headers.
+  x = FOO1_QUOTE(x); // expected-warning {{ambiguous expansion of macro}}
+  // expected-note@Inputs/macro-ambiguity/c/quote/c_quote.h:4 {{expanding this definition}}
+  // expected-note@Inputs/macro-ambiguity/a/quote/a_quote.h:4 {{other definition}}
+
+  x = FOO1_SYSTEM(x);
+
+  x = BAR1_QUOTE(x); // expected-warning {{ambiguous expansion of macro}}
+  // expected-note@Inputs/macro-ambiguity/d/quote/d_quote.h:4 {{expanding this definition}}
+  // expected-note@Inputs/macro-ambiguity/a/quote/a_quote.h:5 {{other definition}}
+
+  x = BAR1_SYSTEM(x);
+
+  x = BAZ1_QUOTE(x); // expected-warning {{ambiguous expansion of macro}}
+  // expected-note@Inputs/macro-ambiguity/a/quote/a_quote.h:6 {{expanding this definition}}
+  // expected-note@Inputs/macro-ambiguity/e/quote/e_quote.h:4 {{other definition}}
+
+  x = BAZ1_SYSTEM(x);
+
+  // Here, we don't even warn on bar because in that cas both b and d are
+  // system modules and so the use of non-system headers is irrelevant.
+  x = FOO2_QUOTE(x); // expected-warning {{ambiguous expansion of macro}}
+  // expected-note@Inputs/macro-ambiguity/c/quote/c_quote.h:5 {{expanding this definition}}
+  // expected-note@Inputs/macro-ambiguity/b/quote/b_quote.h:4 {{other definition}}
+
+  x = FOO2_SYSTEM(x);
+
+  x = BAR2_QUOTE(x);
+
+  x = BAR2_SYSTEM(x);
+
+  x = BAZ2_QUOTE(x); // expected-warning {{ambiguous expansion of macro}}
+  // expected-note@Inputs/macro-ambiguity/b/quote/b_quote.h:6 {{expanding this definition}}
+  // expected-note@Inputs/macro-ambiguity/e/quote/e_quote.h:5 {{other definition}}
+
+  x = BAZ2_SYSTEM(x);
+  return x;
+}