r202475 - Add 'remark' diagnostic type in 'clang'

Tobias Grosser tobias at grosser.es
Mon Apr 28 01:20:59 PDT 2014


Hi Alp,

I am very sorry I did not reply to this email. No good excuses on my side.

On 02/03/2014 14:10, Alp Toker wrote:
> Tests?

We did not had in-tree implementations at that moment, so committing 
tests was difficult. I tested them with Polly.

Diego now finished the -Rpass=inliner support and committed test cases
that test this feature in-tree.

 > VerifyDiagnosticConsumer support?

I submitted a patch to the mailing list.

 > libclang stability?

I will address this in a separate email when resubmitting the patch you 
reverted.

> Trying to emit a 'remark' asserts with "Diagnostic not handled during
> diagnostic buffering!" right now so it doesn't quite seem to be working.

This is only the case in '-verify' mode. remarks itself worked and work
without problems and are now also tested in tree. The 
VerifyDiagnosticConsumer patch fixes this assert.

> Moreover the libclang changes are breaking stable applications here when
> remark diags are encountered, either due to missing handling in language
> bindings, or otherwise when remarks get treated as fatal errors due to
> incorrect severity being reported (clang_getDiagnosticSeverity(diag) >=
> CXDiagnostic_Error).

I will address this in a separate email when resubmitting the patch you 
reverted. As remarks are fully opt-in, the changes did not break any 
existing use case though.

> Both of these require application code using libclang to be rewritten if
> this approach is taken and so should be avoided until remark semantics
> are stable and some compelling use cases have landed.

As inliner and vectorizer remarks are now available we hopefully have a 
better idea of the use cases.

> (While I said "OK" to a specific point, I didn't mean "OK to land" and
> hadn't looked at the patch in depth yet. The changes need to come with a
> couple of tests at the least, and they've yet to be reviewed/LGTM'd by
> the usual maintainers.)

I got an explicit 'The patch itself looks good' from Richard Smith. 
Still, I did not reply to your email in time. Sorry for this! Are there 
any open issues you would like to see addressed.

Cheers,
Tobias



More information about the cfe-commits mailing list