[PATCH] Added new functionality to LLVM C API to use DiagnosticInfo to handle errors

Quentin Colombet qcolombet at apple.com
Fri Apr 11 09:33:12 PDT 2014


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>




More information about the llvm-commits mailing list