Support for 'abstract' modifier for class and spec recommended warnings, issue 375

Support for 'abstract' modifier for class and spec recommended warnings, issue 375
http://code.google.com/p/dart/issues/detail?id=375


R=zundel@google.com
BUG=
TEST=

Committed: https://code.google.com/p/dart/source/detail?r=2181

[zundel, 2011-11-28 23:27:06]

LGTM, but please get someone on the language team to confirm this is what we
really want, since so much of the description of abstract classes is tied up in
non-normative text.

http://codereview.chromium.org/8728001/diff/1/compiler/java/com/google/dart/c...
File
compiler/java/com/google/dart/compiler/resolver/SyntheticDefaultConstructorElement.java
(right):

http://codereview.chromium.org/8728001/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/resolver/SyntheticDefaultConstructorElement.java:1:
package com.google.dart.compiler.resolver;
needs copyright header

http://codereview.chromium.org/8728001/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java (right):

http://codereview.chromium.org/8728001/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:710: // XXX
remove comment

http://codereview.chromium.org/8728001/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:743: // done
remove comment

[gbracha, 2011-11-30 17:29:34]

Hi,

I don't know the codebase nearly well enough to comment, but for the most part
the changes look plausible. However, see my comment about calling factory
constructors of abstract classes. Why is this supposed to be ok?

http://codereview.chromium.org/8728001/diff/1/compiler/javatests/com/google/d...
File
compiler/javatests/com/google/dart/compiler/type/TypeAnalyzerCompilerTest.java
(right):

http://codereview.chromium.org/8728001/diff/1/compiler/javatests/com/google/d...
compiler/javatests/com/google/dart/compiler/type/TypeAnalyzerCompilerTest.java:340:
*/
Why is it ok to call a factory of an abstract class? How is it different from an
ordinary constructor? And where in the spec did you find this exception?

[scheglov, 2011-11-30 17:44:25]

Thank you for comments.

http://codereview.chromium.org/8728001/diff/1/compiler/javatests/com/google/d...
File
compiler/javatests/com/google/dart/compiler/type/TypeAnalyzerCompilerTest.java
(right):

http://codereview.chromium.org/8728001/diff/1/compiler/javatests/com/google/d...
compiler/javatests/com/google/dart/compiler/type/TypeAnalyzerCompilerTest.java:340:
*/
On 2011/11/30 17:29:34, gbracha wrote:
> Why is it ok to call a factory of an abstract class? How is it different from
an
> ordinary constructor? And where in the spec did you find this exception?

  It is just logical to allow call of factory constructor without warning.
  As we think optimistically in Dart, we suppose that developer knows what he
does, so we suppose that factory constructor will create valid instance of some
concrete subclass. When we see using normal constructor for abstract class -
this is obviously bad thing. But for factory - no.

  You are right, there are no exception for this in spec.

  I've just thought that for warnings I can speculate a little what is good and
what is bad. I just think what would be useful for IDE users and what not.
  I'm OK with restoring warning, but probably with different ID, to allow IDE
users disable it separately from using non-factory constructor for abstract
class.