[cfe-commits] r164874 - in /cfe/trunk: include/clang/Lex/PPCallbacks.h include/clang/Lex/PreprocessingRecord.h lib/Frontend/DependencyFile.cpp lib/Frontend/DependencyGraph.cpp lib/Lex/PPDirectives.cpp lib/Lex/PreprocessingRecord.cpp lib/Lex/Prepr
richard at metafoo.co.uk
Mon Oct 8 14:13:27 PDT 2012
On Mon, Oct 8, 2012 at 1:21 PM, Kim Gräsman <kim.grasman at gmail.com> wrote:
> Hi Richard, all,
> On Mon, Oct 1, 2012 at 8:31 PM, Richard Smith <richard at metafoo.co.uk>
> > Yes, I think we should maintain SourceLocation fidelity wherever
> > Clients can convert to the expansion location themselves if they need to.
> > It looks like the CharEnd calculation for the string_literal and
> > angle_string_literal cases is wrong:
> > Filename = getSpelling(FilenameTok, FilenameBuffer);
> > End = FilenameTok.getLocation();
> > CharEnd = End.getLocWithOffset(Filename.size());
> > You can't use getLocWithOffset to find the end of a token this way (it
> > work for trigraphs, at least). This should be using getLocForEndOfToken,
> > except that that apparently doesn't work inside a macro expansion. (That
> > looks like the same problem which is breaking Kim's angled-include-macro
> > case).
> Thanks for the hints!
> There seems to be two problems:
> 1) In the case of literal tokens, CharEnd was previously based on
> Filename.size(). This fails to take trigraphs into account. It looks
> like it works better with FilenameTok.getLength():
> case tok::angle_string_literal:
> case tok::string_literal:
> Filename = getSpelling(FilenameTok, FilenameBuffer);
> End = FilenameTok.getLocation();
> - CharEnd = End.getLocWithOffset(Filename.size());
> + CharEnd = End.getLocWithOffset(FilenameTok.getLength());
> 2) In the case of the less token (triggered by macro expansions that
> result in angled includes), CharEnd was based on End, which had been
> advanced by ConcatenateIncludeName. End already points to the closing
> '>', so I changed it to:
> case tok::less:
> // This could be a <foo/bar.h> file coming from a macro expansion.
> In this
> // case, glue the tokens together into FilenameBuffer and interpret
> if (ConcatenateIncludeName(FilenameBuffer, End))
> return; // Found <eod> but no ">"? Diagnostic already emitted.
> Filename = FilenameBuffer.str();
> - CharEnd = getLocForEndOfToken(End);
> + CharEnd = End.getLocWithOffset(1);
> I've tested these by running my callback impl over a simple .cc file,
> and I get the results I expect, i.e. the FilenameRange contains the
> macro "body" (this is the spelling location, right?);
Both of those changes look correct to me. [IIRC, the 'spelling location' is
what you get by flattening a (hierarchical) source location down to a
(non-hierarchical) location where the character literally appeared in a
source file, and that's not quite what you mean here, but close enough.]
> #define MACRO_TRIGRAPH "trigraph??-empty.h"
> #define MACRO_ANGLED <vadefs.h>
> #define MACRO_QUOTED "empty.h"
> #define MACRO_STRINGIZED(x) #x
> #include MACRO_QUOTED
> #include MACRO_ANGLED
> #include MACRO_TRIGRAPH
> #include MACRO_STRINGIZED(empty.h)
> #include <vadefs.h>
> #include "empty.h"
> (<vadefs.h> is an MSVC header that has no further includes, so as not
> to muddy the results)
> Is there test coverage for this stuff somewhere? I'm not familiar with
> Clang's testing tools yet, but I would make an effort to get this
> under test if I knew where to start...
I can't find any tests for this. I think the best way to proceed would be
to add a PPCallbacksTest.cpp to unittests/Lex.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-commits