[cfe-commits] [PATCH] PR6101: Preprocessor doesn't honor #line directives

[Jordy Rose]

On Mon, 12 Apr 2010 17:03:44 -0700, Chris Lattner <clattner at apple.com>
wrote:
> On Apr 10, 2010, at 10:13 PM, Jordy Rose wrote:
> 
>> PR6101 is a little misleading; #line directives /are/ honored, but not
by
>> PrintPreprocessorOutput.cpp (-E mode). (Unless there is a separate
>> problem
>> on PC Linux.) This patch fixes that by using presumed locations instead
>> of
>> instantiation locations.
>> 
>> There is a bit of a hack to handle the fact that actually entering and
>> exiting source files looks the same as GNU linemarkers. (System header
>> markers and normal "renames" are always from directives.)
>> - Entering a file counts as a line directive if the include location is
>> in
>> the same file as the file itself.
>> - Exiting a file counts as a line directive...if the exit doesn't
happen
>> at the beginning of a line. This is a kludge, and takes advantage of
>> #include eating the newline before loading the next file, but I
couldn't
>> think of a better test that got everything correct (including the exit
>> linemarkers in the predefines).
> 
> Thanks for the patch, I ended up with a slightly different approach in
> r101097.  It turns out that GCC is wildly inconsistent with the exact
> number it uses for line markers and this is the closest I could
reasonably
> get.  I removed the .h part of your testcase because GCC was not
accepting
> it (generating great errors in linemap.c :). 
> 
> -Chris


Ah, this is "correct" but has a little more than we should be printing.
For just preprocessing this file:

static char c;
// EOF

we get:

# 2 "/Users/jordy/Desktop/temp.c"
# 2 "/Users/jordy/Desktop/temp.c" 1
# 2 "<built-in>" 1
# 1 "<built-in>" 3
# 112 "<built-in>" 3
# 1 "<command line>" 1
# 1 "<built-in>" 2
# 2 "/Users/jordy/Desktop/temp.c" 2
# 1 "/Users/jordy/Desktop/temp.c"
static char c;

That is, that extra "+1" is causing it to start files on line 2. It gets
corrected right afterwards, but this is unnecessary noise. That's what the
extra header file was testing (though I should've checked it more carefully
with GCC!) and what this mess was meant to deal with:

if (Reason == PPCallbacks::EnterFile) {
  SourceLocation IncludeLoc = UserLoc.getIncludeLoc();
  if (!IncludeLoc.isValid() || !SourceMgr.isFromSameFile(IncludeLoc, Loc))
    IsLineDirective = false;
} else if (Reason == PPCallbacks::ExitFile) {
  // FIXME replace with something more reliable
  if (UserLoc.getColumn() == 0)
    IsLineDirective = false;
}
// RenameFile and SystemHeaderPragma are always directives

If !IsLineDirective (either of those cases), adding 1 to UserLoc.getLine()
is incorrect -- we're actually entering or exiting a file instead of just
reading a directive. Special-casing the main file isn't correct either; it
also affects #includes.

I certainly don't like these contortions, but without adding an
IsLineDirective parameter to the method, I'm not sure if there's a better
way.

Jordy