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

[Argyrios Kyrtzidis]

On Feb 12, 2014, at 8:49 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:

> 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.

Ah, right.

> 
>> 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.

LGTM!

> 
> 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>*/
> <libclang-report-error-code-for-bad-pch-v2.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140212/07216280/attachment.html>