[PATCH] Added new functionality to LLVM C API to use DiagnosticInfo to handle errors
Quentin Colombet
qcolombet at apple.com
Tue Apr 15 10:37:34 PDT 2014
LGTM.
Thanks Tom.
-Quentin
On Apr 15, 2014, at 10:32 AM, Tom Stellard <tom at stellard.net> wrote:
> Hi Quentin,
>
> Here is an updated patch with the default case. OK to commit?
>
> Thanks,
> Tom
>
> On Fri, Apr 11, 2014 at 09:33:12AM -0700, Quentin Colombet wrote:
>> Hi Tom,
>>
>> This LGTM with one comment:
>> + switch(unwrap(DI)->getSeverity()) {
>> + case DS_Error:
>> + severity = LLVMDSError;
>> + break;
>> + case DS_Warning:
>> + severity = LLVMDSWarning;
>> + break;
>> + case DS_Remark:
>> + severity = LLVMDSRemark;
>> + break;
>> + case DS_Note:
>> + severity = LLVMDSNote;
>> + break;
>> + }
>>
>> You may want to add a default case that default to LLVMDSError in case the header is not in sync with the actual API.
>> That said, I do not know if it can happen.
>>
>> One side question, do we have any API_VERSION macro for the LLVM C API?
>> From the client point of view, how can we know that some specific features are available?
>>
>> Thanks,
>> -Quentin
>> On Apr 10, 2014, at 7:03 AM, Tom Stellard <tom at stellard.net> wrote:
>>
>>> Hi,
>>>
>>> Here is an updated patch that exposes severity and also adds a comment
>>> about using LLVMDisposeMessage.
>>>
>>> -Tom
>>>
>>>
>>> On Sat, Apr 05, 2014 at 06:02:23AM -0700, Eric Christopher wrote:
>>>> *shrug* I take a conservative view on the C API, but since so many
>>>> people want it I'll say OK :)
>>>>
>>>> -eric
>>>>
>>>> On Fri, Apr 4, 2014 at 3:04 PM, Reid Kleckner <rnk at google.com> wrote:
>>>>> Why not expose severity? That seems like one of the more stable aspects of
>>>>> our diagnostics. Besides, it's just another enum that we promise to keep
>>>>> stable.
>>>>>
>>>>>
>>>>> On Fri, Apr 4, 2014 at 10:59 AM, Eric Christopher <echristo at gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> I'd very much prefer not to expose the severity for now. This is a
>>>>>> fairly well thought out, but very new interface and we guarantee
>>>>>> stability of the C interface so we'd be locking the current severity
>>>>>> interface down.
>>>>>>
>>>>>> I'm not, honestly, a huge fan of opening this up to the C API in
>>>>>> general - is there a good reason to do so?
>>>>>>
>>>>>> -eric
>>>>>>
>>>>>> On Fri, Apr 4, 2014 at 10:39 AM, Quentin Colombet <qcolombet at apple.com>
>>>>>> wrote:
>>>>>>> Hi Darren,
>>>>>>>
>>>>>>> This LGTM with two comments:
>>>>>>> - LLVMPrintDiagInfoToString should state that the string has to be freed
>>>>>>> (see inline comment).
>>>>>>> - Should we provide an API to expose the severity as well? What would do
>>>>>>> the
>>>>>>> users of LLVMPrintDiagInfoToString if we do not?
>>>>>>>
>>>>>>> Note that I'm not super familiar with the C API, so you may want to wait
>>>>>>> for
>>>>>>> another review.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> -Quentin
>>>>>>>
>>>>>>> On Apr 2, 2014, at 8:32 PM, Tom Stellard <tom at stellard.net> wrote:
>>>>>>>
>>>>>>> From: Darren Powell <darren.powell at amd.com>
>>>>>>>
>>>>>>> ---
>>>>>>> include/llvm-c/Core.h | 20 ++++++++++++++++++++
>>>>>>> include/llvm/IR/DiagnosticInfo.h | 3 +++
>>>>>>> lib/IR/Core.cpp | 20 ++++++++++++++++++++
>>>>>>> 3 files changed, 43 insertions(+)
>>>>>>>
>>>>>>> diff --git a/include/llvm-c/Core.h b/include/llvm-c/Core.h
>>>>>>> index 50c5e3a..17367ff 100644
>>>>>>> --- a/include/llvm-c/Core.h
>>>>>>> +++ b/include/llvm-c/Core.h
>>>>>>> @@ -124,6 +124,12 @@ typedef struct LLVMOpaquePassRegistry
>>>>>>> *LLVMPassRegistryRef;
>>>>>>> * @see llvm::Use */
>>>>>>> typedef struct LLVMOpaqueUse *LLVMUseRef;
>>>>>>>
>>>>>>> +
>>>>>>> +/**
>>>>>>> + * @see llvm::DiagnosticInfo
>>>>>>> + */
>>>>>>> +typedef struct LLVMOpaqueDiagnosticInfo *LLVMDiagnosticInfoRef;
>>>>>>> +
>>>>>>> typedef enum {
>>>>>>> LLVMZExtAttribute = 1<<0,
>>>>>>> LLVMSExtAttribute = 1<<1,
>>>>>>> @@ -453,6 +459,8 @@ void LLVMEnablePrettyStackTrace(void);
>>>>>>> * @{
>>>>>>> */
>>>>>>>
>>>>>>> +typedef void (*LLVMDiagnosticHandler)(LLVMDiagnosticInfoRef, void *);
>>>>>>> +
>>>>>>> /**
>>>>>>> * Create a new context.
>>>>>>> *
>>>>>>> @@ -467,6 +475,13 @@ LLVMContextRef LLVMContextCreate(void);
>>>>>>> LLVMContextRef LLVMGetGlobalContext(void);
>>>>>>>
>>>>>>> /**
>>>>>>> + * Set the diagnostic handler for this context.
>>>>>>> + */
>>>>>>> +void LLVMContextSetDiagnosticHandler(LLVMContextRef C,
>>>>>>> + LLVMDiagnosticHandler Handler,
>>>>>>> + void *DiagnosticContext);
>>>>>>> +
>>>>>>> +/**
>>>>>>> * Destroy a context instance.
>>>>>>> *
>>>>>>> * This should be called for every call to LLVMContextCreate() or memory
>>>>>>> @@ -474,6 +489,11 @@ LLVMContextRef LLVMGetGlobalContext(void);
>>>>>>> */
>>>>>>> void LLVMContextDispose(LLVMContextRef C);
>>>>>>>
>>>>>>> +/**
>>>>>>> + * Extract string description from DiagnosticInfo.
>>>>>>>
>>>>>>>
>>>>>>> Add a comment here saying that the string should be freed using
>>>>>>> LLVMDisposeMessage.
>>>>>>>
>>>>>>> + */
>>>>>>> +char *LLVMPrintDiagInfoToString(LLVMDiagnosticInfoRef DI);
>>>>>>> +
>>>>>>> unsigned LLVMGetMDKindIDInContext(LLVMContextRef C, const char* Name,
>>>>>>> unsigned SLen);
>>>>>>> unsigned LLVMGetMDKindID(const char* Name, unsigned SLen);
>>>>>>> diff --git a/include/llvm/IR/DiagnosticInfo.h
>>>>>>> b/include/llvm/IR/DiagnosticInfo.h
>>>>>>> index de2a9d8..5fd6636 100644
>>>>>>> --- a/include/llvm/IR/DiagnosticInfo.h
>>>>>>> +++ b/include/llvm/IR/DiagnosticInfo.h
>>>>>>> @@ -193,6 +193,9 @@ public:
>>>>>>> }
>>>>>>> };
>>>>>>>
>>>>>>> +// Create wrappers for C Binding types (see CBindingWrapping.h).
>>>>>>> +DEFINE_SIMPLE_CONVERSION_FUNCTIONS(DiagnosticInfo,
>>>>>>> LLVMDiagnosticInfoRef)
>>>>>>> +
>>>>>>> } // End namespace llvm
>>>>>>>
>>>>>>> #endif
>>>>>>> diff --git a/lib/IR/Core.cpp b/lib/IR/Core.cpp
>>>>>>> index 4f3d8b1..e3b70c7 100644
>>>>>>> --- a/lib/IR/Core.cpp
>>>>>>> +++ b/lib/IR/Core.cpp
>>>>>>> @@ -17,6 +17,8 @@
>>>>>>> #include "llvm/IR/Attributes.h"
>>>>>>> #include "llvm/IR/CallSite.h"
>>>>>>> #include "llvm/IR/Constants.h"
>>>>>>> +#include "llvm/IR/DiagnosticInfo.h"
>>>>>>> +#include "llvm/IR/DiagnosticPrinter.h"
>>>>>>> #include "llvm/IR/DerivedTypes.h"
>>>>>>> #include "llvm/IR/GlobalAlias.h"
>>>>>>> #include "llvm/IR/GlobalVariable.h"
>>>>>>> @@ -76,6 +78,14 @@ LLVMContextRef LLVMGetGlobalContext() {
>>>>>>> return wrap(&getGlobalContext());
>>>>>>> }
>>>>>>>
>>>>>>> +void LLVMContextSetDiagnosticHandler(LLVMContextRef C,
>>>>>>> + LLVMDiagnosticHandler Handler,
>>>>>>> + void *DiagnosticContext) {
>>>>>>> + unwrap(C)->setDiagnosticHandler(
>>>>>>> + LLVM_EXTENSION
>>>>>>> reinterpret_cast<LLVMContext::DiagnosticHandlerTy>(Handler),
>>>>>>> + DiagnosticContext);
>>>>>>> +}
>>>>>>> +
>>>>>>> void LLVMContextDispose(LLVMContextRef C) {
>>>>>>> delete unwrap(C);
>>>>>>> }
>>>>>>> @@ -89,6 +99,16 @@ unsigned LLVMGetMDKindID(const char* Name, unsigned
>>>>>>> SLen)
>>>>>>> {
>>>>>>> return LLVMGetMDKindIDInContext(LLVMGetGlobalContext(), Name, SLen);
>>>>>>> }
>>>>>>>
>>>>>>> +char *LLVMPrintDiagInfoToString(LLVMDiagnosticInfoRef DI) {
>>>>>>> + std::string MsgStorage;
>>>>>>> + raw_string_ostream Stream(MsgStorage);
>>>>>>> + DiagnosticPrinterRawOStream DP(Stream);
>>>>>>> +
>>>>>>> + unwrap(DI)->print(DP);
>>>>>>> + Stream.flush();
>>>>>>> +
>>>>>>> + return strdup(MsgStorage.c_str());
>>>>>>> +}
>>>>>>>
>>>>>>> /*===-- Operations on modules
>>>>>>> ---------------------------------------------===*/
>>>>>>>
>>>>>>> --
>>>>>>> 1.8.3.2
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> llvm-commits mailing list
>>>>>>> llvm-commits at cs.uiuc.edu
>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> llvm-commits mailing list
>>>>>>> llvm-commits at cs.uiuc.edu
>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>>>
>>>>>> _______________________________________________
>>>>>> llvm-commits mailing list
>>>>>> llvm-commits at cs.uiuc.edu
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>
>>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> <0001-VER-2-Added-new-functionality-to-LLVM-C-API-to-use-D.patch>
>>
> <0001-VER-3-Added-new-functionality-to-LLVM-C-API-to-use-D.patch>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140415/66de3630/attachment.html>
More information about the llvm-commits
mailing list