gwtproject / gwt

GWT Open Source Project
http://www.gwtproject.org
1.51k stars 372 forks source link

Add @MustBeLiteral to make GWT.create(Class) less magical and more extensible #2249

Open dankurka opened 9 years ago

dankurka commented 9 years ago

Originally reported on Google Code with ID 2243

Found in GWT Release:
All of them so far (up to GWT 1.5 M1)

Detailed description:

Every now and then I'd like to be able to wrap a GWT.create() call in a
"library" method that invokes GWT.create() and then does something with the
result before returning it.  The limitation that GWT.create() must be
called with a class literal means that such a library method must have the
argument to GWT.create() "baked in", even if the call sites of the library
method could all provide a class literal.  This could be solved in a
controlled, easy-to-explain manner by adding support for @MustBeLiteral to
the compiler.

Rather than GWT.create() being exceptionally magical, it could be reduced
to mostly magical if it were written like this:

public class GWT {

  public static <T> T create(@MustBeLiteral Class<T> klass) {
    // magic
  }
}

The compiler would then have to be changed to enforce that
@MustBeLiteral-annotated parameters must either be literals, or,
recursively, are @MustBeLiteral-annotated parameters of the call site.  You
could then write this:

public class MyFactory {

  public Foo makeFoo(@MustBeLiteral Class<? extends Foo> klass, Object...
parms) {
    Foo ret = GWT.create(klass);

    ret.setParms(parms);

    return ret;
  }
}

I think the benefits are:

 - libraries can wrap GWT.create() in a controlled manner
 - the transitive nature of @MustBeLiteral is pretty easy to grasp
 - GWT.create()'s magicalness is reduced
 - if GWT.create() is extended to support arbitrary arguments
   as in issue 1595, the required new code might be less magical
   (and therefore more straightforward) if the Object... parameter
   were annotated as follows:
   create(@MustBeLiteral Class<T> klass, @MustBeLiteral Object... args);
 - @MustBeLiteral could conceivably be applied not only to method
   parameters, but also final fields and static final fields for
   additional flexibility

The only downside I can think of is:

 - a given call site for GWT.create() would now have to handle multiple
   possible values for its argument

I think the downside can be handled in a number of ways.  The most
straightforward would be to switch on the passed-in class name and
instantiate the appropriate generated source.  You might be able to avoid
the switch by using Javascript's polymorphic abilities if you added
synthesized methods to each of the Class instances so that GWT.create()
calls would compile down to something like this:

GWT.create(klass) becomes

klass.instatiateWithGeneratorFoo()

Where instantiateWithGeneratorFoo() is added to the vtable for each Class
instance that is passed to a generator named GeneratorFoo, and the
implementation for each class just returns an instance of the type defined
by GeneratorFoo applied to that class.

Workaround if you have one:
Ugly code.

Links to the relevant GWT Developer Forum posts:
I first mentioned the idea in this form here:
http://groups.google.com/group/Google-Web-Toolkit/browse_thread/thread/cf36c64ff48b3e19?hl=en
Ray Cromwell mentioned a similar idea in this thread:
http://groups.google.com/group/Google-Web-Toolkit-Contributors/browse_thread/thread/2039c573f2648344

Reported by ispeters on 2008-04-02 19:37:15

dankurka commented 9 years ago
Just had a random thought:

In the future case where this enhancement is already working and developers are
trying to extend the functionality to its logical limit, it'd be cool if reference
types could be defined as being literals if they were "value types" with all their
instance fields defined as final.  For example, I have a class called Rectangle:

public final class Rectangle {

  private final double height;

  private final double width;

  public Rectangle(double width, double height) {
    this.width = width;
    this.height = height;
  }

  public double getArea() {
    return width * height;
  }

  public double getHeight() {
    return height;
  }

  public double getWidth() {
    return width;
  }
}

It would be cool if you could use that like this:

public class Foo {

  public void foo(@MustBeLiteral Rectangle rect) {
    // whatever
  }
}

The restriction on all fields being final would be necessary to enforce the fact that
rect.getWidth(), rect.getHeight(), and rect.getArea() would have to be compile-time
constants for a given rect.

The main use for this feature would be passing such value objects to an extended
GWT.create(), as defined in issue 1603.

Implementing this extended feature would require additional data flow analysis, I
think, because you'd have to make sure that all instances of Rectangle passed to
methods accepting @MustBeLiteral Rectangle parameters are constructed with
(transitively) literal values.

Implementing this feature might make @MustBeLiteral harder to understand because
@MustBeLiteral-annotated parameters would no long have to be literals in the
traditional sense.  Perhaps this problem could be fixed with a better name for the
annotation and some good examples in the documentation.

Ian

Reported by ispeters on 2008-04-02 20:22:39

dankurka commented 9 years ago
Marking this "Planned", meaning that we will seriously consider it, but not as a guarantee
that this will end up 
being implemented as is. We'll evaluate based on the number and importance of the use
cases enabled.

Reported by bruce+personal@google.com on 2008-04-11 02:08:29

dankurka commented 9 years ago
we need this functionality like yesterday!! 

Reported by rakeshw on 2008-05-28 02:22:49

dankurka commented 9 years ago
I like the sound of a compiler error message, assuming there isn't one already.  For
the annotation, it sounds 
nice but is there any other use case than GWT.create?  If GWT.create is the only case
then we should simply add 
the error for GWT.create.

Reported by spoon+personal@google.com on 2008-05-28 14:32:15

dankurka commented 9 years ago
Hey Comment 4, check out
http://groups.google.com/group/Google-Web-Toolkit/browse_thread/thread/1b8fc78504cb3c46
for another use case.  Dead code elimination could be "enforced" in an implementation
of MessageDigest.getInstance("MD5") if getInstance() were declared as
getInstance(@MustBeLiteral String impl).

By the way, how come I can't see the email addresses for comments 2 and 4?  I'm
logged in--isn't that enough to prove I'm not a bot?

Reported by ispeters on 2008-05-30 15:54:28

dankurka commented 9 years ago
Re MessageDigest:  Java lets you pass a computed string into the method, so why wouldn't
the GWT version?  I 
don't think you should use @MustBeLiteral in this case even if GWT supported it.  You
get better optimization if 
you pass a literal, but that's true of many other methods.

IMHO the hunt is still on for a second GWT-specific use case.

Reported by spoon+personal@google.com on 2008-05-30 17:33:59

dankurka commented 9 years ago
Yeah, if GWT were to bundle MessageDigest with its JRE emulation, I would expect its
getInstance() to be able to accept computed strings, but the use case behind the URL
I posted is a particular user who's adding MessageDigest to his application and he
doesn't have to support all possible uses of getInstance()--only those relevant to
his application.  Giving him @MustBeLiteral would allow him to lean on the compiler
and enforce what would otherwise have to be company policy, thus improving his end
user experience, which is a specific, high-priority goal of GWT.

Reported by ispeters on 2008-05-30 18:01:30

dankurka commented 9 years ago
For non-GWT code, you can always use the Java annotation-processing framework.  -Lex

Reported by spoon+personal@google.com on 2008-05-30 18:48:53

dankurka commented 9 years ago

Reported by scottb+legacy@google.com on 2008-08-14 01:46:51

dankurka commented 9 years ago

Reported by bruce+personal@google.com on 2008-10-21 22:02:28

dankurka commented 9 years ago

Reported by scottb@google.com on 2010-02-03 17:03:14

dankurka commented 9 years ago
I have a second use-case. I ran into this discussion while trying to figure out 
how to get the following code to work.

<E extends Enum<E>> E lookupToken( Class<E> c, String tokenInfo )
{
...
return = Enum.valueOf( c, someFunc( tokenInfo ) );
}

String tokenInfo = getTokenInfoFromSomewhere();
switch( lookupToken( Tokens.class, tokenInfo ) )
{
...
}

enum Tokens
{
One,Two,Three;
}

Naturally, this didn't work in GWT. I think the reason might be the Enum.valueOf()

method is "magic" in a way similar to GWT.create() method. The addition of the 
@MustBeLiteral annotation would make this work I think. It looks like the 
@MustBeLiteral annotation will let us use GWT.create() as well as Enum.valueOf() in

libraries.

Reported by cuchaz on 2010-03-14 20:39:47

dankurka commented 9 years ago
Please disregard previous message.

I did manage to get the above code working. Enum.valueOf() works just fine with 
Class<E> c when a class literal is assigned to c. A bug elsewhere in my code caused

someFunc( tokenInfo ) to return an empty string which of course is not a member of

the enum described by c. Unfortunately, Enum.valueOf() throws an incredibly cryptic

error message in this case making that particular problem very hard to diagnose.

Reported by cuchaz on 2010-03-18 04:11:45

dankurka commented 9 years ago
I'm also having a need in this enhancement to use some WidgetsFactory which creates
mvp4g modules (they must be created using GWT.create()) and returns their views using
one single method like 

private Widget createModuleAndReturnView(Class<? extends Mvp4gModule>
widgetModuleClass) {
  Mvp4gModule widgetModule = (Mvp4gModule) GWT.create(widgetModuleClass);
  widgetModule.createAndStartModule();
  return (Widget)widgetModule.getStartView();       
}

(in reality, I have a class that extends Mvp4g module which extends its
functionality, that's why I need this factory)
}}}

Reported by shaman.sir on 2010-05-05 14:03:24

dankurka commented 9 years ago

Reported by rjrjr@google.com on 2011-01-13 03:11:50

dankurka commented 9 years ago
Note to Daniel K.: Please don't mark as AssumedStale :)

Reported by vidal@umich.edu on 2013-05-26 23:29:45

dankurka commented 9 years ago
This is exactly what we discussed at google IO, should we try to land this for GWT 2.6?

Reported by dankurka@google.com on 2013-06-02 15:08:43