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

provide a way to find parameter types of ClassModel #750

Closed
jvasileff opened this issue Sep 16, 2015 · 10 comments
Closed

provide a way to find parameter types of ClassModel #750

jvasileff opened this issue Sep 16, 2015 · 10 comments
Assignees
Milestone

Comments

@jvasileff
Copy link
Member

There doesn't seem to be a replacement for ClassModel.parameterTypes.

I tried without success to obtain either a CallableConstructorDeclaration or a FunctionModel:

shared class MMFoo(String s) {}

shared void mm2testing() {
    print(type(`MMFoo`.defaultConstructor)); // just returns `MMFoo`
    print(`MMFoo`.getCallableConstructors<>()); // empty list
}
@tombentley tombentley modified the milestone: 1.2 Sep 17, 2015
@tombentley
Copy link
Member

I screwed up. Again. I had to remove parameterTypes because not every class has a parameter list these days. But I then forgot I'd done that when I came to add defaultConstructor. Maybe it would suffice to add a ClassInitializer which satisfies FunctionModel, and change the return type of defaultConstructor to FunctionModel?

@FroMage
Copy link
Member

FroMage commented Sep 17, 2015

I don't get it. Didn't we say that classes with initialisers emulate having a single default constructor? That seems wrong:

shared sealed interface ClassWithInitializerDeclaration 
        satisfies ClassDeclaration {
    shared actual default [] constructorDeclarations() => [];
    shared actual default Null defaultConstructorDeclaration => null;

It should have a single constructor, no?

@tombentley
Copy link
Member

We I said a lot of things, over a long period of time, and various things that seemed like good ideas proved not to work. Emulating a default constructor is actually quite difficult because the metamodel generally assumes there's a 1:1 relationship between a tc declaration and a meta declaration (e.g. Metamodel.getOrCreateMetamodel()) which is no longer true if a Class can be a ClassDeclaration or some kind of synthetic ConstructorDeclaration. And if we exposed a synthetic declaration for a class, does the class or the constructor get the annotations? And it would be just wrong, on a conceptual level for a ClassWithInitializerDeclaration to have a ConstructorDeclaration, because a class with a parameter list doesn't have constructors, by definition.

Now at the level of the model (not declaration) it makes a lot more sense to abstract away, as much as possible, the difference. That's partly what defaultConstructor is for.

@FroMage
Copy link
Member

FroMage commented Sep 17, 2015

Well, I don't see why we can't change the definition of class with initialiser to say it's equivalent to class with single default constructor, unless they are conceptually different but I don't see how.

As for it being hard to implement, is it hard or impossible? Because it still sounds like it would be the most convenient way to do it. Same for the model, ATM sure we have ClassModel.defaultConstructor but it returns a ClassModel which is not a Functional so we still can't use it for parameter types.

Now, about annotations, that's a good question. I suppose the answer depends on how annotations apply to constructors vs types, which is still an open question ATM.

@tombentley tombentley self-assigned this Sep 24, 2015
tombentley added a commit that referenced this issue Sep 29, 2015
They're not actually used since it's very difficult to usefully abstract
callable constructors and value constructors since they're very different
and existing abstractions such as Declaration are sufficient.

Part of #750
tombentley added a commit that referenced this issue Sep 29, 2015
* Add ClassDeclaration.defaultConstructor, refined to be non-optional
  on ClassWithInitialiserDeclaration.
* Make parameterDeclarations optional on ClassDeclaration, refined to be non-optional
  on ClassWithInitialiserDeclaration.
* Fix doc on getParameterDeclaration() to clarify it will always return
  null on classes with neither parameter list nor default constructor.
* Add a test for these things
* Fix a couple of broken tests

Part of #750
tombentley added a commit that referenced this issue Sep 29, 2015
tombentley added a commit that referenced this issue Sep 29, 2015
Part of #750
tombentley added a commit that referenced this issue Sep 29, 2015
tombentley added a commit that referenced this issue Sep 29, 2015
Part of #750
@tombentley
Copy link
Member

I've pushed a ctors branch which has a CallableConstructor to represent the parameter list and initializer, likewise declarations. Before asking @chochos to implement this in JS can @jvasileff try it out. It probably needs a bit more testing (the declaration stuff is quite well tested, but the models less so), but hopefully the API is better.

@jvasileff
Copy link
Member Author

I'm not sure what sort of feedback you're looking for, but sticking to the basics:

  1. ``MMFoo.defaultConstructor?.parameterTypes works
  2. type(MMFoo.defaultConstructor); breaks with "com.redhat.ceylon.compiler.java.runtime.metamodel.ModelError: Unsupported declaration type: class com.redhat.ceylon.model.typechecker.model.TypeParameter"
  3. ``MMFoo.getCallableConstructors<>() returns an empty list, but shouldn't?

tombentley added a commit that referenced this issue Sep 30, 2015
@tombentley
Copy link
Member

Thanks @jvasileff, that's perfect. @chochos, would you be able to take a look at the ctors branch?

@chochos
Copy link
Member

chochos commented Sep 30, 2015

sure, just le me fix a compiler bug I found last night

chochos added a commit that referenced this issue Sep 30, 2015
chochos added a commit that referenced this issue Sep 30, 2015
chochos added a commit that referenced this issue Oct 1, 2015
@chochos
Copy link
Member

chochos commented Oct 1, 2015

ready to merge...

tombentley added a commit that referenced this issue Oct 9, 2015
They're not actually used since it's very difficult to usefully abstract
callable constructors and value constructors since they're very different
and existing abstractions such as Declaration are sufficient.

Part of #750
tombentley added a commit that referenced this issue Oct 9, 2015
* Add ClassDeclaration.defaultConstructor, refined to be non-optional
  on ClassWithInitialiserDeclaration.
* Make parameterDeclarations optional on ClassDeclaration, refined to be non-optional
  on ClassWithInitialiserDeclaration.
* Fix doc on getParameterDeclaration() to clarify it will always return
  null on classes with neither parameter list nor default constructor.
* Add a test for these things
* Fix a couple of broken tests

Part of #750
tombentley added a commit that referenced this issue Oct 9, 2015
tombentley added a commit that referenced this issue Oct 9, 2015
Part of #750
tombentley added a commit that referenced this issue Oct 9, 2015
tombentley added a commit that referenced this issue Oct 9, 2015
Part of #750
tombentley added a commit that referenced this issue Oct 9, 2015
chochos added a commit that referenced this issue Oct 9, 2015
chochos added a commit that referenced this issue Oct 9, 2015
@chochos chochos closed this as completed in 6ad16b9 Oct 9, 2015
@tombentley
Copy link
Member

I've rebased and merged this branch.

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

4 participants