[PATCH] Added new functionality to LLVM C API to use DiagnosticInfo to handle errors
Tom Stellard
tom at stellard.net
Fri Apr 11 09:42:40 PDT 2014
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.
>
Sure, one scenario where you could hit the default case is if someone added
a new severity to the c++ API, but did not update the C API.
> 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?
>
Most projects I've seen that use LLVM define their own LLVM_VERSION macro and use
that to determine which features are available.
-Tom
> 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