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

Get rid of Set<ClassInfo> usages #174

Open
mkouba opened this issue Feb 15, 2022 · 3 comments
Open

Get rid of Set<ClassInfo> usages #174

mkouba opened this issue Feb 15, 2022 · 3 comments

Comments

@mkouba
Copy link
Contributor

mkouba commented Feb 15, 2022

I've found 22 usages of Set<ClassInfo> directly in jandex classes org.jboss.jandex.Index and org.jboss.jandex.CompositeIndex. Given the fact that ClassInfo does not implement equals/hashCode it would make sense to refactor these classes, esp. the CompositeIndex where multiple ClassInfo instances representing the same java class may appear.

@Ladicek
Copy link
Contributor

Ladicek commented Feb 28, 2022

Those may actually be correct, because in absence of equals/hashCode, a HashMap essentially degenerates into an IdentityHashMap, which may be desired, especially in case of CompositeIndex. That said, I didn't look too deep, reviewing all the places is certainly a good idea.

@mkouba
Copy link
Contributor Author

mkouba commented Mar 8, 2022

may be desired, especially in case of CompositeIndex

Could you be more specific?

TBH I find the fact that a CompositeIndex may contain multiple ClassInfos (same or different) of the same FQCN and the CompositeIndex.getClassByName(DotName) method just returns the first one found very confusing...

@Ladicek
Copy link
Contributor

Ladicek commented Mar 8, 2022

I have the same feelings about CompositeIndex containing multiple equivalent but different classes, but I can't tell whether that was an intentional decision or not, and at this point, can't even rule out that there are people depending on it :-)

I mean, I agree it should be investigated, but I'm not 100% convinced yet that they all may safely be eliminated.

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

No branches or pull requests

2 participants