Diagnose the expansion of ambiguous macro definitions. This can happen

only with modules, when two disjoint modules #define the same
identifier to different token sequences.

llvm-svn: 165746

clang/include/clang/Basic/DiagnosticGroups.td

@@ -212,6 +212,7 @@ def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">;
 
 // Preprocessor warnings.
 def : DiagGroup<"builtin-macro-redefined">;
+def AmbiguousMacro : DiagGroup<"ambiguous-macro">;
 
 // Just silence warnings about -Wstrict-aliasing for now.
 def : DiagGroup<"strict-aliasing=0">;

clang/include/clang/Basic/DiagnosticLexKinds.td

@@ -228,6 +228,12 @@ def pp_macro_not_used : Warning<"macro is not used">, DefaultIgnore,
 def warn_pp_undef_identifier : Warning<
   "%0 is not defined, evaluates to 0">,
   InGroup<DiagGroup<"undef">>, DefaultIgnore;
+def warn_pp_ambiguous_macro : Warning<
+  "ambiguous expansion of macro %0">, InGroup<AmbiguousMacro>;
+def note_pp_ambiguous_macro_chosen : Note<
+  "expanding this definition of %0">;
+def note_pp_ambiguous_macro_other : Note<
+  "other definition of %0">;
 
 def pp_invalid_string_literal : Warning<
   "invalid string literal, ignoring final '\\'">;

clang/include/clang/Lex/MacroInfo.h

@@ -111,6 +111,10 @@ private:
   /// file.
   bool IsHidden : 1;
 
+  /// \brief Whether the definition of this macro is ambiguous, due to
+  /// multiple definitions coming in from multiple modules.
+  bool IsAmbiguous : 1;
+
    ~MacroInfo() {
     assert(ArgumentList == 0 && "Didn't call destroy before dtor!");
   }
@@ -340,6 +344,13 @@ public:
   /// \brief Set whether this macro definition is hidden.
   void setHidden(bool Val) { IsHidden = Val; }
 
+  /// \brief Determine whether this macro definition is ambiguous with
+  /// other macro definitions.
+  bool isAmbiguous() const { return IsAmbiguous; }
+
+  /// \brief Set whether this macro definition is ambiguous.
+  void setAmbiguous(bool Val) { IsAmbiguous = Val; }
+  
 private:
   unsigned getDefinitionLengthSlow(SourceManager &SM) const;
 };

clang/lib/Lex/MacroInfo.cpp

@@ -32,7 +32,8 @@ MacroInfo::MacroInfo(SourceLocation DefLoc)
     IsAllowRedefinitionsWithoutWarning(false),
     IsWarnIfUnused(false),
     IsPublic(true),
-    IsHidden(false) {
+    IsHidden(false),
+    IsAmbiguous(false) {
 }
 
 MacroInfo::MacroInfo(const MacroInfo &MI, llvm::BumpPtrAllocator &PPAllocator)
@@ -56,7 +57,8 @@ MacroInfo::MacroInfo(const MacroInfo &MI, llvm::BumpPtrAllocator &PPAllocator)
     IsAllowRedefinitionsWithoutWarning(MI.IsAllowRedefinitionsWithoutWarning),
     IsWarnIfUnused(MI.IsWarnIfUnused),
     IsPublic(MI.IsPublic),
-    IsHidden(MI.IsHidden) {
+    IsHidden(MI.IsHidden),
+    IsAmbiguous(MI.IsAmbiguous) {
   setArgumentList(MI.ArgumentList, MI.NumArguments, PPAllocator);
 }
 

clang/lib/Lex/PPMacroExpansion.cpp

@@ -89,15 +89,25 @@ void Preprocessor::addLoadedMacroInfo(IdentifierInfo *II, MacroInfo *MI,
     // Find the end of the definition chain.
     MacroInfo *Prev = StoredMI;
     MacroInfo *PrevPrev;
-    bool Ambiguous = false;
+    bool Ambiguous = StoredMI->isAmbiguous();
+    bool MatchedOther = false;
     do {
       // If the macros are not identical, we have an ambiguity.
-      if (!Prev->isIdenticalTo(*MI, *this))
-        Ambiguous = true;
+      if (!Prev->isIdenticalTo(*MI, *this)) {
+        if (!Ambiguous) {
+          Ambiguous = true;
+          StoredMI->setAmbiguous(true);
+        }
+      } else {
+        MatchedOther = true;
+      }
     } while ((PrevPrev = Prev->getPreviousDefinition()) &&
              PrevPrev->isDefined());
 
-    // FIXME: Actually use the ambiguity information for something.
+    // If there are ambiguous definitions, and we didn't match any other
+    // definition, then mark us as ambiguous.
+    if (Ambiguous && !MatchedOther)
+      MI->setAmbiguous(true);
 
     // Wire this macro information into the chain.
     MI->setPreviousDefinition(Prev->getPreviousDefinition());
@@ -360,7 +370,23 @@ bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier,
       }
     }
   }
-  
+
+  // If the macro definition is ambiguous, complain.
+  if (MI->isAmbiguous()) {
+    Diag(Identifier, diag::warn_pp_ambiguous_macro)
+      << Identifier.getIdentifierInfo();
+    Diag(MI->getDefinitionLoc(), diag::note_pp_ambiguous_macro_chosen)
+      << Identifier.getIdentifierInfo();
+    for (MacroInfo *PrevMI = MI->getPreviousDefinition();
+         PrevMI && PrevMI->isDefined();
+         PrevMI = PrevMI->getPreviousDefinition()) {
+      if (PrevMI->isAmbiguous()) {
+        Diag(PrevMI->getDefinitionLoc(), diag::note_pp_ambiguous_macro_other)
+          << Identifier.getIdentifierInfo();
+      }
+    }
+  }
+
   // If we started lexing a macro, enter the macro expansion body.
 
   // If this macro expands to no tokens, don't bother to push it onto the

clang/lib/Serialization/ASTReader.cpp

@@ -1254,6 +1254,24 @@ void ASTReader::ReadMacroRecord(ModuleFile &F, uint64_t Offset,
   SmallVector<IdentifierInfo*, 16> MacroArgs;
   MacroInfo *Macro = 0;
 
+  // RAII object to add the loaded macro information once we're done
+  // adding tokens.
+  struct AddLoadedMacroInfoRAII {
+    Preprocessor &PP;
+    MacroInfo *Hint;
+    MacroInfo *MI;
+    IdentifierInfo *II;
+
+    AddLoadedMacroInfoRAII(Preprocessor &PP, MacroInfo *Hint)
+      : PP(PP), Hint(Hint), MI(), II() { }
+    ~AddLoadedMacroInfoRAII( ) {
+      if (MI) {
+        // Finally, install the macro.
+        PP.addLoadedMacroInfo(II, MI, Hint);
+      }
+    }
+  } AddLoadedMacroInfo(PP, Hint);
+
   while (true) {
     unsigned Code = Stream.ReadCode();
     switch (Code) {
@@ -1370,8 +1388,9 @@ void ASTReader::ReadMacroRecord(ModuleFile &F, uint64_t Offset,
       }
       MI->setHidden(Hidden);
 
-      // Finally, install the macro.
-      PP.addLoadedMacroInfo(II, MI, Hint);
+      // Make sure we install the macro once we're done.
+      AddLoadedMacroInfo.MI = MI;
+      AddLoadedMacroInfo.II = II;
 
       // Remember that we saw this macro last so that we add the tokens that
       // form its body to it.

clang/test/Modules/Inputs/macros_left.h

@@ -7,6 +7,8 @@
 
 
 #define LEFT_RIGHT_IDENTICAL int
-#define LEFT_RIGHT_DIFFERENT float
+
 #define LEFT_RIGHT_DIFFERENT2 float
 #define LEFT_RIGHT_DIFFERENT3 float
+
+#define LEFT_RIGHT_DIFFERENT float

clang/test/Modules/Inputs/macros_right.h

@@ -1,8 +1,8 @@
 #include "macros_top.h"
 #define RIGHT unsigned short
 
-#undef TOP_RIGHT_REDEF
-#define TOP_RIGHT_REDEF float
+
+
 
 
 
@@ -13,3 +13,5 @@
 #define LEFT_RIGHT_DIFFERENT2 int
 #define LEFT_RIGHT_DIFFERENT3 int
 
+#undef TOP_RIGHT_REDEF
+#define TOP_RIGHT_REDEF float

clang/test/Modules/Inputs/macros_top.h

@@ -2,4 +2,12 @@
 
 #define TOP_LEFT_UNDEF 1
 
+
+
+
+
+
+
+
+
 #define TOP_RIGHT_REDEF int

clang/test/Modules/macros.c

@@ -7,8 +7,14 @@
 // RUN: %clang_cc1 -E -fmodules -x objective-c -fmodule-cache-path %t %s | FileCheck -check-prefix CHECK-PREPROCESSED %s
 // FIXME: When we have a syntax for modules in C, use that.
 // These notes come from headers in modules, and are bogus.
+
 // FIXME: expected-note{{previous definition is here}}
-// FIXME: expected-note{{previous definition is here}}
+// expected-note{{other definition of 'LEFT_RIGHT_DIFFERENT'}}
+// expected-note{{expanding this definition of 'TOP_RIGHT_REDEF'}}
+// FIXME: expected-note{{previous definition is here}} \
+// expected-note{{expanding this definition of 'LEFT_RIGHT_DIFFERENT'}}
+
+// expected-note{{other definition of 'TOP_RIGHT_REDEF'}}
 
 @__experimental_modules_import macros;
 
@@ -109,11 +115,11 @@ void test2() {
   int i;
   float f;
   double d;
-  TOP_RIGHT_REDEF *ip = &i; // FIXME: warning
+  TOP_RIGHT_REDEF *ip = &i; // expected-warning{{ambiguous expansion of macro 'TOP_RIGHT_REDEF'}}
   
   LEFT_RIGHT_IDENTICAL *ip2 = &i;
-  LEFT_RIGHT_DIFFERENT *fp = &f; // FIXME: warning
-  LEFT_RIGHT_DIFFERENT2 *dp = &d; // okay
+  LEFT_RIGHT_DIFFERENT *fp = &f; // expected-warning{{ambiguous expansion of macro 'LEFT_RIGHT_DIFFERENT'}}
+  LEFT_RIGHT_DIFFERENT2 *dp = &d;
   int LEFT_RIGHT_DIFFERENT3;
 }