On Tuesday 30 of July 2013, Eli Friedman wrote:
> On Sun, Jul 28, 2013 at 2:50 AM, Lubos Lunak <[email protected]> wrote:
> > On Monday 01 of July 2013, Eli Friedman wrote:
> >> On Sun, Jun 30, 2013 at 12:01 AM, Lubos Lunak <[email protected]> wrote:
> >> >   Hello,
> >> >
> >> >  could somebody please review and commit the atached patch for pr15610
> >> > (it needs the patch for pr15610 applied first in order to apply
> >> > cleanly). Thank you.
> >> >
> >> > Testcase?
> >
> >  Updated.
>
> This isn't really the right approach; it suppresses the warning when
> the -frewrite-includes flag is used, but not for the output of
> -frewrite-includes, which is what actually matters.

 I disagree (or don't understand what you mean). The -frewrite-includes mode 
is just some auxiliary step that actually has nothing to do with 
preprocessing or compilation, those will be done when the resulting file will 
be compiled. So it should not report or warn about any problem with the file, 
except possible issues related to -frewrite-includes itself, as all those 
reports will be handled when the result is passed to Clang again.

 So if I understand the above correctly as you saying that -Wunused-macros 
should be blocked for -frewrite-includes as well as for the follow-up 
compilation, then this is not right, the warning should be there for the 
actual compilation, just like it is there when compiling the original file 
directly.

 Updated patch that applies to current trunk attached. I've also modified it a 
bit to show that the warnings are blocked for DisableMacroExpansion (since if 
macros are not going to be expanded, they obviously will be unused, so maybe 
it's clearer this way).

> See the thread "[PATCH] Correctly check for the main file in the
> presence of line markers".

 I don't find that one relevant (except that with current trunk line markers 
block -Wunused-macros completely, so this is now broken 
with -frewrite-includes, but I'll submit a patch for that).

-- 
 Lubos Lunak
From 18e3e991248d4bd2d4d0360c5a2649106d693b81 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= <[email protected]>
Date: Sun, 28 Jul 2013 11:32:55 +0200
Subject: [PATCH] do not emit -Wunused-macros warnings in -frewrite-includes
 mode (PR15614)

-frewrite-includes calls PP.SetMacroExpansionOnlyInDirectives() to avoid
macro expansions that are useless in that mode, but this can lead
to -Wunused-macros false positives. As -frewrite-includes does not emit
normal warnings, block -Wunused-macros too.
---
 lib/Lex/PPDirectives.cpp                  | 5 +++++
 test/Frontend/rewrite-includes-warnings.c | 7 ++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/Lex/PPDirectives.cpp b/lib/Lex/PPDirectives.cpp
index 2245086..e80720e 100644
--- a/lib/Lex/PPDirectives.cpp
+++ b/lib/Lex/PPDirectives.cpp
@@ -694,6 +694,9 @@ public:
       pp->DisableMacroExpansion = false;
   }
   ~ResetMacroExpansionHelper() {
+    restore();
+  }
+  void restore() {
     PP->DisableMacroExpansion = save;
   }
 private:
@@ -805,6 +808,7 @@ void Preprocessor::HandleDirective(Token &Result) {
 
     // C99 6.10.3 - Macro Replacement.
     case tok::pp_define:
+      helper.restore();
       return HandleDefineDirective(Result, ImmediatelyAfterTopLevelIfndef);
     case tok::pp_undef:
       return HandleUndefDirective(Result);
@@ -2137,6 +2141,7 @@ void Preprocessor::HandleDefineDirective(Token &DefineTok,
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
+      !DisableMacroExpansion &&
       Diags->getDiagnosticLevel(diag::pp_macro_not_used,
           MI->getDefinitionLoc()) != DiagnosticsEngine::Ignored) {
     MI->setIsWarnIfUnused(true);
diff --git a/test/Frontend/rewrite-includes-warnings.c b/test/Frontend/rewrite-includes-warnings.c
index 1fb98db..c4f38ed 100644
--- a/test/Frontend/rewrite-includes-warnings.c
+++ b/test/Frontend/rewrite-includes-warnings.c
@@ -1,4 +1,9 @@
-// RUN: %clang_cc1 -verify -Wall -Wextra -E -frewrite-includes %s
+// RUN: %clang_cc1 -verify -Wall -Wextra -Wunused-macros -E -frewrite-includes %s
 // expected-no-diagnostics
 
+// PR14831
 #pragma GCC visibility push (default)
+
+// PR15614
+#define FOO 1
+int foo = FOO;
-- 
1.8.1.4

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to