[cfe-commits] [Patch 2 of 7] -verify fixes and enhancement
Jordan Rose
jordan_rose at apple.com
Tue Jul 3 17:35:44 PDT 2012
On Jul 3, 2012, at 4:22 PM, Jordan Rose wrote:
>
> On Jul 3, 2012, at 3:23 PM, Andy Gibbs wrote:
>
>>>> 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'm afraid it isn't possible to do an assertion since it is very possible for
>> a test-case writer to provide an invalid line number, for example:
>>
>> // expected-error@ {{...}}
>> // expected-error at 0 {{...}}
>> // expected-error at -3 {{...}}
>>
>> (the third will be invalid if the line number of the directive is 3 or less).
>
> In that case, I'd still prefer to treat these as errors, with a possible exception of "@0" meaning "(frontend)". "@" and "@-3" (or "@5000" or "@+50000") seem like things we can catch up front.
Oops, I see that's already been taken care of. Wait, so why do we need the additional check at the end? Referring to this part of the patch:
> @@ -352,14 +384,18 @@
>
> SmallString<256> Fmt;
> llvm::raw_svector_ostream OS(Fmt);
> for (DirectiveList::iterator I = DL.begin(), E = DL.end(); I != E; ++I) {
> Directive& D = **I;
> - if (D.Location.isInvalid() || !SourceMgr)
> + if (D.DiagnosticLoc.isInvalid() || !SourceMgr)
> OS << "\n (frontend)";
> else
> - OS << "\n Line " << SourceMgr->getPresumedLineNumber(D.Location);
> + OS << "\n Line " << SourceMgr->getPresumedLineNumber(D.DiagnosticLoc);
> + if (!D.DirectiveLoc.isInvalid() && D.DirectiveLoc != D.DiagnosticLoc)
> + OS << " (directive at "
> + << SourceMgr->getFilename(D.DirectiveLoc) << ":"
> + << SourceMgr->getPresumedLineNumber(D.DirectiveLoc) << ")";
> OS << ": " << D.Text;
> }
>
> Diags.Report(diag::err_verify_inconsistent_diags)
I see that it's not /your/ isInvalid() originally, but I don't think anything should break if you take it away. ...probably.
More information about the cfe-commits
mailing list