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

[Alp Toker]

On 11/12/2013 18:20, Quentin Colombet wrote:
> 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.

Just for reference, these types of callback adapters tend to get called 
External* on the clang side.

e.g. ExternalSemaSource, ExternalIdentifierLookup, 
ExternalHeaderFileInfoSource.

Might be a good naming convention to follow.

Personally I find them quite useful for trying things quickly, but I 
didn't have a chance to see what the impact is in your patch yet.

Alp.



> 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
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-- 
http://www.nuanti.com
the browser experts