[cfe-dev] [PATCH] libclang: report error code for bad PCH files

[Dmitri Gribenko]

On Wed, Feb 12, 2014 at 4:25 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com> wrote:
> On Feb 12, 2014, at 6:36 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
>
> On Wed, Feb 12, 2014 at 11:04 AM, Dmitri Gribenko <gribozavr at gmail.com>
> wrote:
>
> On Wed, Feb 12, 2014 at 2:05 AM, Argyrios Kyrtzidis <kyrtzidis at apple.com>
> wrote:
>
> This seems more complicated than it’s worth, particularly since we do add a
> new API that the caller must be aware of, anyway.
>
> How about for the functions that don’t support detailed error codes we just
> introduce a new variant that does support error codes ?
> Clients interested in the error codes can move to calling the new function
> (whether we should deprecate the existing one or not is a different
> question).
>
>
> This does seem cleaner.  With this approach, we will have to introduce
> three new APIs, mirroring the following:
>
> clang_createTranslationUnitFromSourceFile
> clang_createTranslationUnit
> clang_parseTranslationUnit
>
>
> clang_createTranslationUnitFromSourceFile is just
> "clang_parseTranslationUnit minus options", we don't need a variant for
> that.

OK, removing it.

> While these APIs already return error codes:
>
> clang_reparseTranslationUnit
> clang_indexSourceFile
>
>
> Are you considering clang_indexTranslationUnit ?

I think it can not fail because of AST deserialization.  The TU should
already exist before calling this function.

> I have implemented this approach in the attached patch.
>
>
> Did you forget to attach it ?
>
>  The new APIs
> are just suffixed with "2", for example,
> clang_createTranslationUnit2(), because of a lack of imagination and
> to clearly show that the two parallel APIs are essentially the same.
>
>
> Um, "clang_createTranslationUnit_Ext" ? I don't have a strong preference,
> it's your call.
>
>
> I did not deprecate the old APIs, we can just treat them as
> "convenience" wrappers.
>
> I was thinking about changing the return type of
> clang_reparseTranslationUnit and clang_indexSourceFile to 'enum
> CXErrorCode', but I am not 100% that it is an ABI-compatible change on
> all platforms (it is compatible on x86-64, I think), so I did not go
> this way.
>
>
> That's fine.
>
>
> Please review.
>
>
> I need a patch! :-)

Sorry, attaching an updated patch now.

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libclang-report-error-code-for-bad-pch-v2.patch
Type: application/octet-stream
Size: 40970 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140212/7eee2a8d/attachment.obj>