[PATCH] Add warning capabilities in LLVM (backend part), Take 2

[Quentin Colombet]

Hi Renato,

On Dec 11, 2013, at 5:06 AM, Renato Golin <renato.golin at linaro.org> wrote:

> On 11 December 2013 12:52, Hal Finkel <hfinkel at anl.gov> wrote:
>>>> For what do you imagine that DiagnosticInfoOther will be used?
>>> I was thinking plugins.
> 
> I think it should be easy for third-party implementations (not-yet or
> never-to-be merged code) to have their own non-clashing handlers,
> which ends up as deriving from the base class only on your tree and
> make use of it only by your code.
If everyone agrees we do not need the DiagnosticInfoOther, I am fine removing it.
However, beyond the plugin case, I thought it would have been a good class to experiment things (like vectorizer information on why something has not been optimized using a DS_Note) or to report a richer message than InlineAsm without creating a specific subclass for each pattern.

> 
> 
>> 1. enum DiagnosticKind should end with DK_FirstPluginKind
> 
> This is ok, because plugins can use/reuse the same enum values, but it
> makes it hard for keeping trees in sync. Worst still, if you use two
> plugins that happen to use the same enum values, we'll only find out
> if they both run on the same binary and report an error.
> 
> Open source plugins can have their values added to the main tree,
> closed source ones cannot.
I think Hal’s suggestion is perfectly fine. The diagnostic ID in the plugins will be set at runtime using the getNextAvailablePluginDiagnosticKind(). Therefore, I do not see any problem here, except maybe thread safety when updating the next available diagnostic kind.

> 
> 
>> 2. Add a global function: int getNextAvailablePluginDiagnosticKind() which returns an incremented static variable (which starts with DK_FirstPluginKind).
> 
> I think we can start simple, and add a DK_PluginKind and make all
> plugins differentiate themselves in the error message,
The only difference I see here with the current DK_Other, is the name in the enum :).

> or even having
> a customized payload for the C-API which passes a PDK_Kind (Plugin
> Diagnostic Kind). But I think this needs more discussion.
For now, I think Hal’s suggestion is a good start. That said, I was not looking in addressing the problem adding new diagnostics at runtime. Thus, we may need another thread to discuss that.
I do not think that was block the current proposal.

What do you think?

Cheers,
Quentin
> 
> cheers,
> --renato