On 08/25/2012 08:45 AM, Gregory Szorc wrote:
I agree with the sentiment of wanting increased compatibility, but I'm
hesitant that this change is the right answer.
Hi Gregory,
thanks for the feedback.
IMO the version compatibility story should be "use the same revision
number for both libclang and the Python bindings." If the revision of
libclang and the Python bindings varies, there is a potential for them
to be out of sync, for backwards incompatibility to cause
unexpected/undefined behavior, etc. This is dangerous. (I would
actually like to have a feature in the Python bindings where they
(optionally) refuse to talk to a libclang that was built from a
different revision of the source tree than the bindings.) If we were
to commit this patch, we are effectively saying that we /think/ the
bindings /might/ be compatible with your version of libclang. But, we
really don't know what features are supported or if they will work
correctly. If we were to take this patch, we'd also need a
compatibility story. What if a user of Clang 2.9 complains about the
current trunk bindings not working? Are we obligated to support them?
What about a 3.0 user? 3.1? Where do you draw the line? We have a
clear line today: use the same version control revision. This patch
removes that and introduces a lot of ambiguity.
You seem to have a strong position when you state that the only approach
that should be used is: 'use the same revision than clang'
I understand your goal of keeping the python bindings in sync with
libclang. As you explained, in an ideal world the python bindings mirror
the features of libclang, they are tested before every release and they
are released _and_ installed with clang. I believe our own goal (and
also what we claim to support) should be to get as close as possible to
such an ideal world.
Unfortunately we are not in an ideal world. In the last releases
(2.8/2.9/3.0/3.1) the python bindings have been lacking behind libclang,
they have not been properly tested, have sometimes been buggy and they
have also never been installed. We can definitely improve on this by
adding more test cases (e.g. code completion) or by installing the
python bindings with clang. However, I honestly doubt we will manage to
develop libclang and the python bindings in sync. In the last years,
features to the python bindings have mostly been added (and tested) when
a tool using such features was developed. This happened often after
releases, meaning older libclang.so versions often support a feature,
but the python bindings from that point did not yet support it.
Tools that use the python bindings need to work in this non-ideal world.
clang_complete (but also SublimeClang [1]) never followed the 'use the
same revision than clang' version rule. Instead they have been bundling
their own version of the python bindings since they started. Like this
they are able to work with many of the existing clang installations,
which - in case of smaller plugins - is a big plus.
clang_complete e.g. currently bundles cindex.py bindings from around
clang 3.0. This allows it to provide code completion with clang 3.0 and
newer. Unfortunately the current bindings fail to load as soon as a
libclang function does not exist. This means clang_complete does not
support clang 2.8 / 2.9 and it can also not move to newer bindings and
their features without breaking support for clang 3.0.
For code completion the only reason of incompatibility is the
initialization. With the proposed patch applied, the most recent
cindex.py version can provide both code completion and error
highlighting for clang 2.8, 2.9, 3.0, 3.1 and trunk. What I want to say:
This patch is not only about some small performance improvements, but it
enables the use of the current python bindings on a wide range of
libclang versions.
However, this is probably what you are afraid of: Using the very same
python binding for a wide range of clang versions. Something that is not
officially supported and that is not covered by test cases. I understand
that there is some risk and that there may be compatibility issues.
However, I don't believe this is a no-go. For code completion the single
cindex.py approach worked out well. This is becasue libclang itself is
very stable and changes to it are generally additions of new
functionality. The semantics of existing functions do not change. There
may still be backward compatibility issues in some border cases, but in
general newer bindings are highly backward compatible. Obviously, such
compatibility claims are not checked for larger parts of cindex.py and I
don't believe the libclang developers should spent time on compatibility
testing or should feel themselves forced to remain compatible. However,
I don't see a problem, if external users test for backward compatibility
and take advantage of backward compatibility, if it is exists.
I understand that you are trying to enable users of old Clang to reap
the recent performance improvements of the Python bindings without
having to upgrade Clang. This is a noble goal. However, this is risky.
Instead, I think this decision should be left to downstream users.
Backporting individual patches is relatively simple. And, the test
suite does give some peace of mind (although the code complete APIs
don't have tests, sadly). Alternatively, if Clang maintained its
release branches post-release, performance patches like the recent
code complete ones would likely be obvious candidates for backport. If
nothing else, some power user could maintain her own branch of
bindings with backported trunk changes that she guarantees to work
with libclang X.Y. I believe that the trunk bindings should only
provide compatibility with trunk libclang.
Who are the downstream users? And what decisions does this patch take
that should be left to the downstream users?
Regarding maintaining post release branches for each release: I think it
is a frightening idea. clang and LLVM have no release branches, because
they add a lot of maintenance overhead. Asking each project to maintain
post-release branches individually will cause even more overhead. For
cases where this overhead can not be avoided, we may have no choice, but
I don't see any reason to force every user to maintain post release
branches. Automatic code completion works well with a single cindex.py
file and the projects that bundle cindex.py for this use case perform
happily the compatibility testing. They are in some way the power users
you are talking about. Do you see a problem if they decide a single
cindex.py works well for them?
Even if we apply this patch, we could still look into some way to give
the user optionally stronger guarantees for his python bindings.
Checking if the bindings and the installed libclang.so are in sync is
one way (we need to cover here also the private Apple releases, a simple
version number checker does not work). Another idea is to check
compatibility based on features. We could have simple tests that check
if a specific libclang version supports feature X (e.g. the
CompilationDatabase). In case the version Apple cut supports that
feature, we would return yes, otherwise no. In case an unsupported
feature would be used anyway, the python bindings could throw a
descriptive exception.
To subsume: I think there are different use cases and this patch
improves a important one.
Cheers
Tobi
[1] https://github.com/quarnster/SublimeClang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
