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(shadow): fix the creation of a partial shadow class #2040

Merged
merged 2 commits into from
Jun 9, 2018

Conversation

tdurieux
Copy link
Collaborator

@tdurieux tdurieux commented Jun 6, 2018

It is a little bit dirty, do you have better idea how to do it?

Fix #2038

visitTypeParameter(generic);
}
} catch (Throwable ignore) {
// partial classpath
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we catch more exact exception? Such error handling may cause big problems during future debugging.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do this

visitTypeReference(CtRole.INTERFACE, anInterface);
try {
for (Type anInterface : clazz.getGenericInterfaces()) {
visitTypeReference(CtRole.INTERFACE, anInterface);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to try/catch this line only. So some interfaces are correctly created and some are ignored. Actually it process all interfaces until first which is not on classpath.

Do the same in all cycles of this method, please.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot the Error is thrown by clazz.getGenericInterfaces()

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and I did not succeed to find a way to get a partial list :/

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So then try/catch only clazz.getGenericSuperclass(). So the exceptions thrown in visitTypeReference are not catched

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have the motivation to do it, right now.
There is probably 50 invocations to protect in this class.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it can be written in nice way using lambda:
for (Type anInterface : ignoreMissingClass(()->clazz.getGenericInterfaces()))
where ignoreMissingClass will return empty collection in case of exception

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clazz.getGenericInterfaces() returns an array not a collection.
I don't see a way to do it nicely :/

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<T> T[] ignoreMissingClass(Supplier<T[]> supl) {
  try {
     return supl.get();
   } catch (Throwable e) {
      //TODO check if it is expected problem, else rethrow
      return EMPTY_ARR;
   }
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the problem is to create EMPTY_ARR, because EMPTY_ARR has to be different for each T

try {
for (Annotation annotation : clazz.getDeclaredAnnotations()) {
visitAnnotation(annotation);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This for cycle seems to be copy past here by mistake. It is already above. Please fix it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand it is the only loop on clazz.getDeclaredAnnotations() for visitEnum(Class clazz).
Did I miss something?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See line 142-144 in this file

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not the same method

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. First one processes Interface and second one processes Enum. You are right. It is correct. Thanks for explanation.

@surli surli mentioned this pull request Jun 7, 2018
@monperrus
Copy link
Collaborator

LGTM.

@monperrus monperrus merged commit 439d0c4 into INRIA:master Jun 9, 2018
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.

3 participants