On Sunday 13 of April 2014, Richard Smith wrote:
> The check for MainFileID and accompanying test LGTM.
>
> Can you explain what the check for PredefinesFileID is for?
Without that check, the output can look e.g. like this:
=====
# 1 "<built-in>" 1
# 1 "a.cpp"
void f() {}
=====
Which is wrong. Only now that I look at it, what's wrong is rather that
there's no closing "2" linemarker for <built-in>.
So how about this attached patch (on top of the previous one that had the
predefines check removed)?
> AFAICS, there's
> no test for that part of the change, and I'm concerned that we could end up
> attributing lines in that FileID to some other file.
The patch includes a test specifically for the predefines file, although that
theoretically shouldn't be necessary, since there's no output from the
predefines file anyway (OutputContentUpTo() skips it).
> On Fri, Apr 4, 2014 at 3:09 AM, Lubos Lunak <[email protected]> wrote:
> > The -frewrite-includes flag incorrectly uses at the beginning a
> > linemarker for the main file with the "1" flag which suggests that the
> > contents have been in fact included from another file. This for example
> > disables -Wunused-macros, which warns only for macros from the main file:
> >
> > $ cat a.cpp
> > #define FOO 1
> > $ clang++ -E -frewrite-includes a.cpp | clang++ -Wall -Wunused-macros -x
> > c++ -c -
> > $ clang++ -Wall -Wunused-macros -c a.cpp
> > a.cpp:1:9: warning: macro is not used [-Wunused-macros]
> > #define FOO 1
> > ^
> > 1 warning generated.
> >
> > The attached patch fixes the code to start the resulting file with the
> > correct linemarker.
--
Lubos Lunak
From 92ea65facd7dcb07f44339a8eede8874dd66795b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= <[email protected]>
Date: Thu, 1 May 2014 16:43:03 +0200
Subject: [PATCH] do not use "1" for line marker for the predefines "file"
either
Similar to r207764.
---
lib/Rewrite/Frontend/InclusionRewriter.cpp | 6 +++++-
test/Frontend/rewrite-includes-cli-include.c | 9 +++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
create mode 100644 test/Frontend/rewrite-includes-cli-include.c
diff --git a/lib/Rewrite/Frontend/InclusionRewriter.cpp b/lib/Rewrite/Frontend/InclusionRewriter.cpp
index ad8328e..d6a434d 100644
--- a/lib/Rewrite/Frontend/InclusionRewriter.cpp
+++ b/lib/Rewrite/Frontend/InclusionRewriter.cpp
@@ -250,6 +250,10 @@ void InclusionRewriter::CommentOutDirective(Lexer &DirectiveLex,
do {
DirectiveLex.LexFromRawLexer(DirectiveToken);
} while (!DirectiveToken.is(tok::eod) && DirectiveToken.isNot(tok::eof));
+ if (&FromFile == PredefinesBuffer) {
+ // OutputContentUpTo() would not output anything anyway.
+ return;
+ }
OS << "#if 0 /* expanded by -frewrite-includes */" << EOL;
OutputContentUpTo(FromFile, NextToWrite,
SM.getFileOffset(DirectiveToken.getLocation()) + DirectiveToken.getLength(),
@@ -353,7 +357,7 @@ bool InclusionRewriter::Process(FileID FileId,
StringRef EOL = DetectEOL(FromFile);
// Per the GNU docs: "1" indicates entering a new file.
- if (FileId == SM.getMainFileID())
+ if (FileId == SM.getMainFileID() || FileId == PP.getPredefinesFileID())
WriteLineInfo(FileName, 1, FileType, EOL, "");
else
WriteLineInfo(FileName, 1, FileType, EOL, " 1");
diff --git a/test/Frontend/rewrite-includes-cli-include.c b/test/Frontend/rewrite-includes-cli-include.c
new file mode 100644
index 0000000..ba96039
--- /dev/null
+++ b/test/Frontend/rewrite-includes-cli-include.c
@@ -0,0 +1,9 @@
+// RUN: not %clang_cc1 -verify -E -frewrite-includes -include %S/Inputs/rewrite-includes2.h %s -o - | FileCheck -strict-whitespace %s
+main_file_line
+// CHECK: {{^}}# 1 "<built-in>"{{$}}
+// CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|\\\\)}}rewrite-includes2.h" 1{{$}}
+// CHECK-NEXT: {{^}}included_line2{{$}}
+// CHECK-NEXT: {{^}}# 1 "<built-in>" 2{{$}}
+// CHECK-NEXT: {{^}}# 1 "{{.*}}rewrite-includes-cli-include.c"{{$}}
+// CHECK-NEXT: FileCheck
+// CHECK-NEXT: {{^}}main_file_line{{$}}
--
1.8.4.5
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
