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