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

Compute invocation candidates with different strategies seems to be broken #586

Closed
seandenigris opened this issue Aug 3, 2015 · 23 comments

Comments

@seandenigris
Copy link
Contributor

Originally reported on Google Code with ID 586

When you try to import a smalltalk model and select a another strategy than the default
one, you get some errors

Reported by cy.delaunay on 2011-04-12 14:29:04

@seandenigris
Copy link
Contributor Author

I fixed bugs that came from Famix. Now there seems to be some issues from RoelTyper
(When I try to import a big model). For example :

   TypeCollector class >> typeInstvar:ofClassWithLookup:
    "self typeInstvar: #origin ofClassWithLookup: Quadrangle"
    | theClass |
    theClass := aClass.
    [theClass isNil not
        and: [(theClass instVarNames includes: var) not]]
        whileTrue: [theClass := theClass superclass].
    theClass isNil
        ifTrue: [^ ExtractedType new].
    ^ self typeInstvar: var ofClass: theClass

If it does not find the instance variable, it returns 'ExtractedType new' , whereas
ExtractedType is an abstract class and should not implement 'new'

Reported by cy.delaunay on 2011-04-12 14:57:30

@seandenigris
Copy link
Contributor Author

The importer now check that a FAMIXVariable is an instance variable before asking roeltyper
to cpmpute its type. Avoid to end up in the case where roeltyper does not find the
variable.

There is now another problem:
If you try to import all 'Morphic' packages in moose, and select 'compute for any kind
of receiver', it raise an error

Reported by cy.delaunay on 2011-04-13 08:30:39

@seandenigris
Copy link
Contributor Author

Please do not forget to write tests for these cases.

Reported by tudor.girba on 2011-04-13 09:24:52

  • Labels added: Component-SmalltalkImporter, Milestone-4.4

@seandenigris
Copy link
Contributor Author

It would be good to get this nicely working. Cyrille, do you have time to look into
this?

Reported by tudor.girba on 2011-04-30 23:50:53

@seandenigris
Copy link
Contributor Author

I gave a solution that make the example working, but I didn't fixed the problem.
I flaged the possible bug. Added an example to run (as comment) to highlight the problem.

I added a statement for this specific case,  where I return self (not able to fix anything).


From moose point of view, it seems to work nicely. 
I think we just let roelTyper less precise for those specific cases (that means a set
of possible types bigger than if the code was running correctly).  

Reported by cy.delaunay on 2011-05-02 10:02:19

@seandenigris
Copy link
Contributor Author

Cyrille can you be more precise ?
where is the flag ? the example ? the statement ?

We should fix the bug !!

Reported by jannik.laval on 2011-05-03 07:02:25

@seandenigris
Copy link
Contributor Author

All is in TypeCollector >> handleAssignment:forTmp:
I think it's a roel typer issue

Reported by cy.delaunay on 2011-05-03 07:43:32

@seandenigris
Copy link
Contributor Author

Ok thank you.
If it is, could you contact the developer ?

Reported by jannik.laval on 2011-05-03 07:54:24

@seandenigris
Copy link
Contributor Author

I asked this question on the Pharo mailing list, but I got no answers. The RoelTyper
issues can be reproduced on several classes. To see the classes that raise it and the
corresponding errors, you can run:

classes := Dictionary new.
Object withAllSubclassesDo: [:each |
      [(TypeCollector typeInstvarsOfClass: each )] on: Error do: [:error | classes
at: each put: error ]].
classes inspect

Reported by tudor.girba on 2011-05-04 16:40:24

@seandenigris
Copy link
Contributor Author

Any chance someone could look at this one?

Reported by tudor.girba on 2011-05-08 21:11:00

@seandenigris
Copy link
Contributor Author

I tried, but no result.
I do not know where to begin, as I do not know RoelTyper

Reported by jannik.laval on 2011-05-09 13:14:07

@seandenigris
Copy link
Contributor Author

There is still the version RoelTyper-cyrilledelaunay.83
in www.squeaksource.com/RoelTyper that does not concretly fix the problem, but just
skip the problematic cases. 

Reported by cy.delaunay on 2011-05-09 13:39:00

@seandenigris
Copy link
Contributor Author

I tried to use RoelTyper-cyrilledelaunay.83 but it does not work. Try:

- Load the last RoelTyper-cyrilledelaunay.83
- Import AST-Core
- Check the RoelTyper inference

and you will get an error.

Reported by tudor.girba on 2011-05-09 21:05:15

@seandenigris
Copy link
Contributor Author

Issue 626 has been merged into this issue.

Reported by tudor.girba on 2011-05-10 15:37:03

@seandenigris
Copy link
Contributor Author

Cyrille, could you take a look at this?

Reported by tudor.girba on 2011-05-12 19:28:38

@seandenigris
Copy link
Contributor Author

What do you mean by 'check the roelTyper inference' ?
What I do is:
- Load the last RoelTyper-cyrilledelaunay.83
- Import AST-Core using 'compute for any kind of receiver'

And the model is well imported without error. From that point, what should I do to
get the error ?

Reported by cy.delaunay on 2011-05-13 08:18:53

@seandenigris
Copy link
Contributor Author

I meant to check the "Compute type of attributes (using RoelTyper)" checkbox.

You will get the error also with the RoelTyper-cyrilledelaunay.83.

Reported by tudor.girba on 2011-05-13 08:34:17

@seandenigris
Copy link
Contributor Author

I added a test:

MooseSmalltalkImporterRoelTyperTest>>#testASTCore

Name: Moose-Tests-SmalltalkImporter-Core-TudorGirba.23
Author: TudorGirba
Time: 13 May 2011, 10:34:20 am
UUID: aa6c3196-3946-45c9-8e9d-260fa3ea89be
Ancestors: Moose-Tests-SmalltalkImporter-Core-janniklaval.22

added test to expose the roel typer problem when computing types of attributes

Reported by tudor.girba on 2011-05-13 08:35:02

@seandenigris
Copy link
Contributor Author

indeed :)

Reported by cy.delaunay on 2011-05-13 08:40:27

@seandenigris
Copy link
Contributor Author

Like previous time, I was just able to implement something that skip the problem. RoelTyper-cyrilledelaunay.84.

If the collection is empty, it returns. This make the test pass 

Reported by cy.delaunay on 2011-05-13 09:12:48

@seandenigris
Copy link
Contributor Author

Thanks. It looks like the issue is fixed and it does not throw errors anymore. We do
not know about the correctness anymore. 

One problem we have to look for is that the fix was done in RoelTyper, and thus it
can have other repercussions in the Pharo-world.

Reported by tudor.girba on 2011-05-13 14:38:48

  • Status changed: Fixed

@seandenigris
Copy link
Contributor Author

Cyrille can you explain what was the problem?

Reported by stephane.ducasse on 2011-05-13 19:15:49

@seandenigris
Copy link
Contributor Author

I don't know what is the problem, I just put a '^self' when encountering problematic
case (the empty collection). Like my previous change ,  I think it does not break anything,
but just let RoelTyper less precise for some cases (that are not working)

Reported by cy.delaunay on 2011-05-16 13:05:17

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

1 participant