[cfe-commits] [PATCH] cindex.py - Make the compatibility check in cindex.py explicit

[Tobias Grosser]

On 09/05/2012 10:55 PM, Gregory Szorc wrote:
> On 9/5/12 1:29 PM, Tobias Grosser wrote:
>> On 09/05/2012 09:07 PM, Gregory Szorc wrote:
>>> I like it! Thank you for doing this.
>>>
>>> Specifically, I like how it defaults to strict and users must explicitly
>>> choose to pull out a foot gun.
>>>
>>> There's probably potential for hooking up clang_getClangVersion() to
>>> make the compatibility check even stricter. I'm think that the Python
>>> bindings would contain a minimal libclang version. This way, if function
>>> semantics ever change between libclang releases (I know the API is
>>> supposed to be backwards compatible, but you never know), we can detect
>>> that. Don't let this hold up committing this patch though.
>>
>> Thanks, committed in r163238.
>>
>> Regarding using clang_getClangVersion(). The comment says:
>>
>>  * \brief Return a version string, suitable for showing to a user, but
>>  *        not intended to be parsed (the format is not guaranteed to be
>>  *        stable).
>>
>> So parsing it to check the version sounds dangerous? We may need to
>> establish another way to check for the libclang version? Maybe using a
>> function, that just gives a version number?
> Adding a function to get the version of libclang is a good idea. We
> could go so far as to have libclang and the Python bindings store the
> SVN revision they were built with. If there is a mismatch, it aborts.
> This is probably too far for most users, especially since the Python
> bindings aren't packaged by many distros today. I think a simple
> major.minor comparison should be sufficient.
>
> That being said, I think I brought this up a while back (not sure if it
> was on this list or IRC) and there was some push back. Apparently some
> thought that because libclang is supposed to be backwards compatible
> that a "get version" API was silly. My memory could be wrong here.

My personal concern is how to get this right in presence of vendor 
versions. The version scheme of Apple may conflict with the versions of 
the opensource libclang. Users may want to target both of them.

Another way to go could be to detect the "version" of libclang in terms 
of the available features. Basically, we detect which functions are 
exposed and based on this we detect the features that we can enable. My 
guess would be that each incompatibility probably came with some new 
functions. Based on their presence/absence we could detect if a feature 
is in sync.

>> Another idea, I was playing with can be seen in the attached patch. I
>> did not finish thinking about it, so I don't yet submit it as an
>> official patch, but you may want to have a look.
> That's a great idea! User-friendly and actionable error messages are
> always good.

Great. The patch still needs some love . We need e.g. to decide if we 
want the version info being part of a separate table or if it should be 
merged with the method signature table. What do you think?

Tobi