Re: [cfe-commits] [Patch 2 of 7] -verify fixes and enhancement

[Andy Gibbs]

On Monday, July 02, 2012 8:54 PM, Andy Gibbs wrote:

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

just keeping an 'unsigned ExpectedLine' around?

silly to me, especially because this isn't free. Is there any problem with

that the filename is displayed where an expected-* check is not matched to
a diagnostic. [...snip...]

The reason for doing this was because early on Richard asked for the ability to include a file name match [...snip...] Plus you'll see in the diagnostics

of this additional feature at the next stage.

The attached patch leaves this unchanged, therefore, pending the implementation

I see we do check if the location is invalid; I think it's okay to make

the command line.

that an assertion. These checks should never come from or refer to lines on

a test-case writer to provide an invalid line number, for example:

// expected-error@ {{...}}
// expected-error@0 {{...}}
// expected-error@-3 {{...}}

I'm afraid it isn't possible to do an assertion since it is very possible for (the third will be invalid if the line number of the directive is 3 or less).

+  StandardDirective(const SourceLocation &DirectiveLoc,
+                    const SourceLocation &DiagnosticLoc,
+                    const std::string &Text, unsigned Count)
+    : Directive(DirectiveLoc, DiagnosticLoc, Text, Count) { }

StringRef. :-)

Done! :o)

+        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...

And I have!

So attached is the new "part 2" patch.  Ready for commit?

Thanks
Andy

verify-part2.diff
Description: Binary data

_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits