[PATCH] Add warning capabilities in LLVM (backend part), Take 2
alp at nuanti.com
Thu Dec 12 10:09:11 PST 2013
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,
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.
> 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?
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
the browser experts
More information about the llvm-commits