Fix a nasty corner case that Neil noticed in PR1900, where we would

incorrectly apply the multiple include optimization to files with
guards like:

#if !defined(x) MACRO

where MACRO could expand to different things in different contexts.
Thanks Neil!

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@45716 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/Lex/Preprocessor.cpp b/Lex/Preprocessor.cpp
index baabd8ee00b167b415aeabdfe47dc38ef3082657..accccc4fbcd3cd39e56288f296cbb873ac1a6be2 100644 (file)
--- a/Lex/Preprocessor.cpp
+++ b/Lex/Preprocessor.cpp
@@ -784,6 +784,10 @@ bool Preprocessor::isNextPPTokenLParen() {
 /// expanded as a macro, handle it and return the next token as 'Identifier'.
 bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier, 
                                                  MacroInfo *MI) {
+  // If this is a macro exapnsion in the "#if !defined(x)" line for the file,
+  // then the macro could expand to different things in other contexts, we need
+  // to disable the optimization in this case.
+  if (CurLexer) CurLexer->MIOpt.ExpandedMacro();
   
   // If this is a builtin macro, like __LINE__ or _Pragma, handle it specially.
   if (MI->isBuiltinMacro()) {

diff --git a/include/clang/Lex/MultipleIncludeOpt.h b/include/clang/Lex/MultipleIncludeOpt.h
index 83b995b4855fde1a43175b6bc597dadee02f5a94..08bb259fda9622c95b810880a93f6272df07014b 100644 (file)
--- a/include/clang/Lex/MultipleIncludeOpt.h
+++ b/include/clang/Lex/MultipleIncludeOpt.h
@@ -29,12 +29,23 @@ class MultipleIncludeOpt {
   /// to false, that way any tokens before the first #ifdef or after the last
   /// #endif can be easily detected.
   bool ReadAnyTokens;
+
+  /// ReadAnyTokens - This is set to false when a file is first opened and true
+  /// any time a token is returned to the client or a (non-multiple-include)
+  /// directive is parsed.  When the final #endif is parsed this is reset back
+  /// to false, that way any tokens before the first #ifdef or after the last
+  /// #endif can be easily detected.
+  bool DidMacroExpansion;
   
   /// TheMacro - The controlling macro for a file, if valid.
   ///
   const IdentifierInfo *TheMacro;
 public:
-  MultipleIncludeOpt() : ReadAnyTokens(false), TheMacro(0) {}
+  MultipleIncludeOpt() {
+    ReadAnyTokens = false;
+    DidMacroExpansion = false;
+    TheMacro = 0;
+  }
   
   /// Invalidate - Permenantly mark this file as not being suitable for the
   /// include-file optimization.
@@ -53,18 +64,30 @@ public:
   // If a token is read, remember that we have seen a side-effect in this file.
   void ReadToken() { ReadAnyTokens = true; }
   
+  /// ExpandedMacro - When a macro is expanded with this lexer as the current
+  /// buffer, this method is called to disable the MIOpt if needed.
+  void ExpandedMacro() { DidMacroExpansion = true; }
+  
   /// EnterTopLevelIFNDEF - When entering a top-level #ifndef directive (or the
   /// "#if !defined" equivalent) without any preceding tokens, this method is
   /// called.
+  ///
+  /// Note, we don't care about the input value of 'ReadAnyTokens'.  The caller
+  /// ensures that this is only called if there are no tokens read before the
+  /// #ifndef.  The caller is required to do this, because reading the #if line
+  /// obviously reads in in tokens.
   void EnterTopLevelIFNDEF(const IdentifierInfo *M) {
-    // Note, we don't care about the input value of 'ReadAnyTokens'.  The caller
-    // ensures that this is only called if there are no tokens read before the
-    // #ifndef.
-    
     // If the macro is already set, this is after the top-level #endif.
     if (TheMacro)
       return Invalidate();
     
+    // If we have already expanded a macro by the end of the #ifndef line, then
+    // there is a macro expansion *in* the #ifndef line.  This means that the
+    // condition could evaluate differently when subsequently #included.  Reject
+    // this.
+    if (DidMacroExpansion)
+      return Invalidate();
+    
     // Remember that we're in the #if and that we have the macro.
     ReadAnyTokens = true;
     TheMacro = M;

diff --git a/test/Preprocessor/mi_opt.c b/test/Preprocessor/mi_opt.c
new file mode 100644 (file)
index 0000000..96029dc
--- /dev/null
+++ b/test/Preprocessor/mi_opt.c
@@ -0,0 +1,11 @@
+// RUN: not clang -fsyntax-only %s
+// PR1900
+// This test should get a redefinition error from m_iopt.h: the MI opt 
+// shouldn't apply.
+
+#define MACRO
+#include "mi_opt.h"
+#undef MACRO
+#define MACRO || 1
+#include "mi_opt.h"
+

diff --git a/test/Preprocessor/mi_opt.h b/test/Preprocessor/mi_opt.h
new file mode 100644 (file)
index 0000000..a82aa6a
--- /dev/null
+++ b/test/Preprocessor/mi_opt.h
@@ -0,0 +1,4 @@
+#if !defined foo MACRO
+#define foo
+int x = 2;
+#endif