Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix class ordering when propagating type annotations #290

Merged
merged 1 commit into from Dec 2, 2022

Conversation

Ladicek
Copy link
Collaborator

@Ladicek Ladicek commented Dec 2, 2022

To propagate type annotations across nested classes, we sort the entire set of indexed classes so that outer classes come before inner classes. We don't really care about ordering between classes that are not enclosed in one another. In other words, if we consider the equivalence relation of "be enclosed in" (where we consider a class to be enclosed in itself, so that the relation is reflexive and hence indeed equivalence), we must only sort the equivalence classes of this relation, individually. That is, we define partial order on classes.

However, to perform that sort, we use the sorting routine in the JDK, which uses the order defined by a Comparator, and that order must be total. In this commit, we define a total order between classes like so:

  1. For each pair of classes A and B:
  2. if A and B have the same name, they are considered equal;
  3. if A is enclosed in B, A is considered greater than B;
  4. if B is enclosed in A, A is considered less than B;
  5. otherwise, A and B are sorted lexicographically based on their name.

The Comparator we use to sort the indexed classes implements this order.

Fixes #289

To propagate type annotations across nested classes, we sort the entire
set of indexed classes so that outer classes come before inner classes.
We don't really care about ordering between classes that are not enclosed
in one another. In other words, if we consider the equivalence relation
of "be enclosed in" (where we consider a class to be enclosed in itself,
so that the relation is reflexive and hence indeed equivalence), we must
only sort the equivalence classes of this relation, individually. That is,
we define partial order on classes.

However, to perform that sort, we use the sorting routine in the JDK, which
uses the order defined by a `Comparator`, and that order must be total.
In this commit, we define a total order between classes like so:

1. For each pair of classes A and B:
2. if A and B have the same name, they are considered equal;
3. if A is enclosed in B, A is considered greater than B;
4. if B is enclosed in A, A is considered less than B;
5. otherwise, A and B are sorted lexicographically based on their name.

The `Comparator` we use to sort the indexed classes implements this order.
@Ladicek Ladicek added this to the 3.0.5 milestone Dec 2, 2022
@Ladicek Ladicek merged commit 886c2a1 into smallrye:main Dec 2, 2022
@Ladicek Ladicek deleted the fix-class-topological-order branch December 2, 2022 13:24
@Ladicek Ladicek self-assigned this Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jandex-maven-plugin throws IllegalArgumentException: Comparison method violates its general contract
1 participant