[cfe-commits] [Patch 2 of 7] -verify fixes and enhancement
Andy Gibbs
andyg1001 at hotmail.co.uk
Mon Jul 2 11:54:52 PDT 2012
On Monday, July 02, 2012 7:28 PM, Jordan Rose wrote:
> On Jun 30, 2012, at 2:12 PM, Andy Gibbs wrote:
>
> Translating the expected line number in and out of a SourceLocation seems
> silly
> to me, especially because this isn't free. Is there any problem with just
> keeping
> an 'unsigned ExpectedLine' around?
The reason for doing this was because early on Richard asked for the ability
to include
a file name match, e.g. of the form:
expected-error at file:line {{ ... }}
meaning that storing a SourceLocation made more sense to me than a
filename/line
number pair. Plus you'll see in the diagnostics that the filename is
displayed where
an expected-* check is not matched to a diagnostic. This means that an
expected-*
check can be in a different file (which, btw, is already the case in a
number of test-
cases) and the diagnostic output from -verify will inform the coder where
the
expected-* directive is held (which may not be the same file as the
diagnostic
occurs in). At the point where we do actually support the full file:line
syntax, a
number of these test-cases will need to be amended to handle the fact that
they
use a directive in one file to match a diagnostic in another... (but that's
a story for
another day).
> I see we do check if the location is invalid; I think it's okay to make
> that an
> assertion. These checks should never come from or refer to lines on the
> command line.
I'll have a look into this tomorrow when I'm in front of the code again.
> + StandardDirective(const SourceLocation &DirectiveLoc,
> + const SourceLocation &DiagnosticLoc,
> + const std::string &Text, unsigned Count)
> + : Directive(DirectiveLoc, DiagnosticLoc, Text, Count) { }
>
> StringRef. :-)
No problems! (and also in your patch 1 comment too).
> + unsigned CurrentLine = SM.getSpellingLineNumber(Pos, &Invalid);
> + if (!Invalid && PH.Next(Line) && (FoundPlus || Line <
> CurrentLine)) {
> + if (FoundPlus) CurrentLine += Line;
> + else CurrentLine -= Line;
> + ExpectedLoc = SM.translateLineCol(SM.getFileID(Pos),
> CurrentLine, 1);
> + }
>
> Nitpick: modifying something called "CurrentLine" confused me for a
> second,
> even though it's obvious what you're doing. "ExpectedLine"?
I don't mind nitpicks -- I'll change this too...
Thanks
Andy
More information about the cfe-commits
mailing list