These are chat archives for ceylon/ceylon-ide-eclipse

13th
May 2016
Gavin King
@gavinking
May 13 2016 10:19
[ceylon-compile] /Users/gavin/ceylon-ide-common/source/com/redhat/ceylon/ide/common/typechecker/IdePhasedUnit.ceylon:76: error: type of member must be assignable to type of refined member 'createUnit' in 'PhasedUnit': 'Unit' is not assignable to 'TypecheckerUnit'
[ceylon-compile]     shared actual default Unit createUnit() {
WAT?
Did you break it, @davidfestal ?
David Festal
@davidfestal
May 13 2016 10:20
no
I'm not aware of that
but didn't we accept a pull request that was touching to this recently ?
Gavin King
@gavinking
May 13 2016 10:22
ah, yeah, we did
David Festal
@davidfestal
May 13 2016 10:23
would it be related to this MichaelNedzelsky/ceylon@ac3c644 ?
FYI I'm deep into IntelliJ API to find the bast to plugin the Ceylon model update into IntelliJ lifecycle
Gavin King
@gavinking
May 13 2016 10:23
perhaps he did not try to build the IDE
David Festal
@davidfestal
May 13 2016 10:24
probably
I don't even have the Eclipse plugin project opened
to gain some memory my main eclipse ...
but if it's urgent I can have a look
Have to go to eat for a while anyway
Gavin King
@gavinking
May 13 2016 10:25
oh yeah, dammit
his patch works by narrowing the type of createUnit() and getUnit()
OK, so this is the kind of thing I’m talking about @davidfestal !
shared actual default BaseIdeModuleSourceMapper moduleSourceMapper 
        => unsafeCast<BaseIdeModuleSourceMapper>(super.moduleSourceMapper);

shared actual default BaseFileVirtualFile unitFile 
        => unsafeCast<BaseFileVirtualFile>(super.unitFile);

shared actual default BaseFolderVirtualFile srcDir 
        => unsafeCast<BaseFolderVirtualFile>(super.srcDir);
wtf man?!
there’s no reason for that
David Festal
@davidfestal
May 13 2016 10:31
I heard your remarks last time already
and asked you to create an issue for me toclean all this
I just cannot do it right now
and woulld like to do it myself
in order to ponder each case, and justify (or attempt to justify) the cases when I still need it
I've added it into my personnal list of tasks
just after I've finished with the current idea inetgration stuff
Gavin King
@gavinking
May 13 2016 10:32
ok
David Festal
@davidfestal
May 13 2016 10:33
if you open an issue, I'll clean what I can clean
and then we'll be able to discuss the rest, and I'll try to take the time to better explain the constraints that led me to do things like this, OK ?
Gavin King
@gavinking
May 13 2016 10:43
oh gawd no this is a mess
David Festal
@davidfestal
May 13 2016 10:44
what ?
Gavin King
@gavinking
May 13 2016 10:44
we have to roll back that patch
David Festal
@davidfestal
May 13 2016 10:44
why ?
Gavin King
@gavinking
May 13 2016 10:44
the IDE does some really awful stuff here
it refines createUnit() to do something nasty
and then adds a newUnit() method
David Festal
@davidfestal
May 13 2016 10:44
wait
Gavin King
@gavinking
May 13 2016 10:45
which has lots of implementations which do not return TypecheckerUnit
it’s all an incredible mess
David Festal
@davidfestal
May 13 2016 10:45
this code has been there for years afaik even in Java
all the implementation return some sub-class of TypecheckerUnit afaik
how is it a problem ?
Gavin King
@gavinking
May 13 2016 10:47
well they don’t return that type
David Festal
@davidfestal
May 13 2016 10:47
a sub type yes
and what is the problem ?
Gavin King
@gavinking
May 13 2016 10:48
they all do this:
shared actual TypecheckerUnit newUnit() => 
        object satisfies ModelServicesConsumer<NativeProject, NativeResource, NativeFolder, NativeFile>{
        }.modelServices.newEditedSourceFile(this);
David Festal
@davidfestal
May 13 2016 10:48
Wait
Gavin King
@gavinking
May 13 2016 10:48
i.e. create a class with no members that creates an object
David Festal
@davidfestal
May 13 2016 10:48
No no wait wait please
Gavin King
@gavinking
May 13 2016 10:48
now it appears that this actually is a TypecheckerUnit
thankfully
so perhaps it’s not that bad
David Festal
@davidfestal
May 13 2016 10:48
This instanciation is a very special case
related to the lifecycle of these classes inheriited from the Java part
that was creating a cumbersome NPE
because of uninitliazed reified type parameters
I've added a comment on the related modelServices methods
There's also a (good IMO) reason for this
Gavin King
@gavinking
May 13 2016 10:51
the good reason is it’s crap
David Festal
@davidfestal
May 13 2016 10:51
It's Crap that we didn't start this in ceylon fully yes
the most crapppy things in all this design
come from the fact that we have to extend an alrdy-existing java project
that we cannot change at our convenience since it is used in other Java contexts (such as the compiler backend)
The reasons behind many of these bad surprises you have in this code usually are of this kind
of course there's also probably my incompetence ;-)
incompetence at finding better ways to tackle these strong constraints at least
Gavin King
@gavinking
May 13 2016 10:56
ok, so i am cleaning this up
it’s not a matter of competence or incompetence
it’s a matter of this is the kind of thing that happens in large systems
but it’s still necessary to point out crap things when one comes across them
otherwise they stay crap
David Festal
@davidfestal
May 13 2016 10:57
but please be sure that it doesn't break anything. It was made so to fix a cumbersome reified generics bug
open an issue for me to uncrap my own work myself !
and feel free to indicate a better way
or a pull request at least, so that I would be aware of what is changed
Gavin King
@gavinking
May 13 2016 10:58
i’m indicating it right now in the next thing i push
David Festal
@davidfestal
May 13 2016 10:58
while I'm doing something else
I'm 100% on Idea currently :-(
pfff
Gavin King
@gavinking
May 13 2016 13:01
@davidfestal is there a good reason that a Unit should not have a hard ref back to its PhasedUnit?
David Festal
@davidfestal
May 13 2016 13:02
yes,
Gavin King
@gavinking
May 13 2016 13:02
ok
David Festal
@davidfestal
May 13 2016 13:02
to avoid memory leaks
Gavin King
@gavinking
May 13 2016 13:02
what i figured
but what particular leak?
David Festal
@davidfestal
May 13 2016 13:02
because PhasedUnit contains Tree.CompilationUnit
and Tree.CompilationUnit has a unit afair
"no reference from the model to the AST" !
I heard long ago ;-)
these weak references are only for optimization
and at the time I had setup this (years ago)
this allowed significant performance imrovement in the overall IDE usage
as you had noticed (afair)
Gavin King
@gavinking
May 13 2016 13:05
well these XxxxUnit things aren’t actually model classes
i’m not sure what they are
David Festal
@davidfestal
May 13 2016 13:05
what ?
Gavin King
@gavinking
May 13 2016 13:05
but they’re definitely not part of the model
David Festal
@davidfestal
May 13 2016 13:05
they are refined version of "Unit"
more precisely of TypecheckerUnit
TypecheckerUnit -> IdeUnit -> JavaUnit or CeylonUnit, etc ...
each one encapsulating or exposing the additional behavior that is specific to it
The same principle is true for PhasedUnit -> IdePhasedUnit -> ProjectSourceFile / ExternalPhasedUnit etc...
Gavin King
@gavinking
May 13 2016 13:07
right, but they’re not part of the model
David Festal
@davidfestal
May 13 2016 13:08
what do you call the model ?
Gavin King
@gavinking
May 13 2016 13:08
they’re part of an architecturally separate component
that extremely unfortunately, happens to hang off Unit in the inheritance tree
David Festal
@davidfestal
May 13 2016 13:08
they are part of the IDE Ceylon model
no, that's not right
Gavin King
@gavinking
May 13 2016 13:08
right, but they’re nothing to do with the typechecker model
David Festal
@davidfestal
May 13 2016 13:09
yes, they are
Gavin King
@gavinking
May 13 2016 13:09
the typechecker doesn’t use anything defined below Unit
they don’t refine anything in Unit
David Festal
@davidfestal
May 13 2016 13:09
Yeah, but look
Gavin King
@gavinking
May 13 2016 13:09
they’re just a convenient place to stick stuff
David Festal
@davidfestal
May 13 2016 13:09
why does the MModelLoader also use Units ?
Gavin King
@gavinking
May 13 2016 13:09
?
David Festal
@davidfestal
May 13 2016 13:09
because a unit is what contains declarations
the model loader also creates units to contain the lazy declarations it creates
this is the same concept
the only thing is
for example: typechecker units don't need to give access to the related AST and PhasedUnit that served to create them
sure, because the typechecker itself is not interested in any action on these units apart from making them (along with declarations) available for the backend or further generation tooling
however the IDE will wait more from a unit, because from this ceylon model element, it will need to come back to the AST (for navigation for example)
hence the weak references from Units to PhasedUnits
another example:
typechecker unit objects
David Festal
@davidfestal
May 13 2016 13:15
have a full path and relative path members
good, because the typechecker only manage ceylon declarations that are under a given number of source directory
but is never interested in knowing in which source directory it is contained
This message was deleted
Another example: the IDE allows loading Ceylon declarations lazily by the model loader from binary archives
such declarations are located in typechecker units
that , in this case, represent the class file from which they were loaded
David Festal
@davidfestal
May 13 2016 13:21
and here again, for these units, this has a signification to allow retrieving the file relative path of the Ceyon source file that generated this binary class inside the CAR
all the XXXXUnit simply describe different sort of units (as the typechecker knows them, whic is a unit of compilation that contains declarations annd has a path in the source directory structure)
seems rather logical to manage this by inheritance
a Java unit (set of declaration contained in the same file) is a sort of Unit that contains Java declarations
and of course it can expose different infomations than Ceylon source units or binary Ceylon units or Java class unit
David Festal
@davidfestal
May 13 2016 13:28
@gavinking : I don't understand where the design problem is here
but maybe once again it's my all object-oriented, all by inheritance tendency... Though in this case I cannot see things differently.
Gavin King
@gavinking
May 13 2016 14:35
Right, there is waaaaay too much inheritance here
David Festal
@davidfestal
May 13 2016 14:36
there were too much sure, especially, as you explained to both Bastien and me some weeks ago, when we were trying to manage the IDE environment inside the common objects
and you have really helped me by saying this, and I already started fixing this
and created the IDE-related services
decoupled from CeylonProject, etc ...
that, finally
will not have to be refined according the the IDE platform
But in the case of Unit-derived classes, I din't agree with this analysis
to me it seems that inheriting to try pushing the external environment inside the classes is bad
it's what I had done with CeylonProject / that had many formal methods
that's not the same use case for XXXUnitsand XXXPhasedUnits
To me an inheritance tree is justified here
But sure, even in these classes I might have introduced things related to external services
David Festal
@davidfestal
May 13 2016 14:41
in which case I will clean these details
wihtout dropping the inheritance tree itself which is justified IMO
David Festal
@davidfestal
May 13 2016 15:05
@gavinking : hard to review this on GitHub only, according to all the changes in formatting also. I'll do it deeply as soon as I've reopened my Eclipse project and pulled everything
Gavin King
@gavinking
May 13 2016 15:09
What does “loading language module packages” do?
David Festal
@davidfestal
May 13 2016 15:09
where ?
Gavin King
@gavinking
May 13 2016 15:09
eclipse is getting stuck on that when building the SDK
David Festal
@davidfestal
May 13 2016 15:10
it seems to be new
never noticed that
it should be quite straightforward