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.


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.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to