Bug 521552

Comment 1 Stephan Herrmann 2017-08-29 13:12:22 EDT

Some random findings:


There is prior art in this area which looks hackish to me:

AnnotationBinding.buildMarkerAnnotationForMemberType(char[][], ModuleBinding, LookupEnvironment)

featuring this comment:
// since this is a member type name using '$' the return binding is a
// problem reference binding with reason ProblemReasons.InternalNameProvided


OTOH, there are only few calls to this getType()-method within the compiler proper. Perhaps, BinaryModuleBinding.getUses() is actually misguided in using this method. By analogy with use in BinaryTypeBinding, maybe s.t. from the getTypeFromConstantPoolName family of methods is more suitable?

When trying this, I think you'd have to compensate for another idiosyncrasy: while most binary info is handle in bytecode format ("Ljava/lang/Object;"), IModule.uses() seems to use source format throughout, even in its incarnation as classfmt.ModuleInfo. Or does JVMS actually not mandate binary format for "uses"?? I see no handling of leading/trailing 'L' and ';'.

BTW, are annotations allowed on service directives?

Comment 2 Sasikanth Bharadwaj 2017-08-30 05:18:45 EDT

(In reply to comment #1)
My 1 1/2 cents :-)
> Some random findings:
> 
> 
> There is prior art in this area which looks hackish to me:
> 
> AnnotationBinding.buildMarkerAnnotationForMemberType(char[][], ModuleBinding,
> LookupEnvironment)
> 
> featuring this comment:
> // since this is a member type name using '$' the return binding is a
> // problem reference binding with reason ProblemReasons.InternalNameProvided
> 
> 
> OTOH, there are only few calls to this getType()-method within the compiler
> proper. Perhaps, BinaryModuleBinding.getUses() is actually misguided in using
> this method. By analogy with use in BinaryTypeBinding, maybe s.t. from the
> getTypeFromConstantPoolName family of methods is more suitable?
> 
Yes
> When trying this, I think you'd have to compensate for another idiosyncrasy:
> while most binary info is handle in bytecode format ("Ljava/lang/Object;"),
> IModule.uses() seems to use source format throughout, even in its incarnation as
> classfmt.ModuleInfo. Or does JVMS actually not mandate binary format for
> "uses"?? I see no handling of leading/trailing 'L' and ';'.
> 
Looks like a leftover from the time when we did not have separation of source and binary modules. Should be changed 
> BTW, are annotations allowed on service directives?
No

Comment 3 Stephan Herrmann 2017-08-30 09:59:41 EDT

(In reply to Sasikanth Bharadwaj from comment #2)
> (In reply to comment #1)
> > OTOH, there are only few calls to this getType()-method within the compiler
> > proper. Perhaps, BinaryModuleBinding.getUses() is actually misguided in using
> > this method. By analogy with use in BinaryTypeBinding, maybe s.t. from the
> > getTypeFromConstantPoolName family of methods is more suitable?
> > 
> Yes

Are the two of you able to proceed down this road?

Comment 4 Stephan Herrmann 2017-08-30 19:50:57 EDT

suggestion: 
   mod.environment.getTypeFromCompoundName(..)
(after making that method public) - assuming that a URB is acceptable, is it? Otherwise, Main should perhaps resolve it on its own.

See also bug 521568 comment 5

Comment 5 Jay Arthanareeswaran 2017-08-31 05:46:59 EDT

Could be a silly question, anyone knows why we don't allow looking up nested binary types?

Comment 6 Stephan Herrmann 2017-08-31 09:32:42 EDT

(In reply to Jay Arthanareeswaran from comment #5)
> Could be a silly question, anyone knows why we don't allow looking up nested
> binary types?

You're questioning a premordial decision: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/LookupEnvironment.java?id=be6c0d208933ac936a6ccb6c66b03d3da13e3796 search for package1.A

In other words: I haven't a clue :) and Philipe may be the only person on this planet who could possibly have a clue, if he remembered every move from before Eclipse became Eclipse. In this case I don't even blame him for the commit comment "*** empty log message ***" ;p

That said, it feels like the method was designed with source syntax in mind, so nested types would be referred to by '.' separated names.

No test in neither compiler.regression suite nor AllJavaModelTests seems to ever touch the line in question.

Ergo: very good question! :)

Comment 7 Jay Arthanareeswaran 2017-09-01 04:14:25 EDT

Two more points to strengthen the point that that code is no longer relevant:

1. That method is an internal API, so if all clients within JDT Core can reliably use it, we are fine.
2. All our tests pass after removing that code.

Comment 8 Eclipse Genie 2020-04-22 05:54:37 EDT

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug.

If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.