Merge: class annotation `autoinit` to explicitly lists initializers.
authorJean Privat <jean@pryen.org>
Sat, 21 Feb 2015 06:04:31 +0000 (13:04 +0700)
committerJean Privat <jean@pryen.org>
Sat, 21 Feb 2015 06:04:31 +0000 (13:04 +0700)
A suggestion of @Morriar is to use a class-level annotation `autoinit` to list initializers.
It should ne rarely used, in case of conflict for instance, or to restrict the number of initializers (but this could be fragile).

A step towards #785

~~~nit
class A
  var a: Int
  var b: String
end

class B
   autoinit c, b, a
   super A
   var c: Bool
end

var a = new A(10, "foo")
var b = new B(true, "foo", 10)
~~~

Some issues remains, I my require your input on these

* why I am required to put `autoinit` before the `super` clause?
  because the grammar is ugly.
  Maybe I should find a way to enable the `autoinit` clause everywhere in the body of the class?
  This is a grammar issue that can be solved independently I think.
* what are the validity of initializers?
  The first commit is used to discriminate properties that are initializers and those that are not.
  I am not sure if restricting what can be used an initializer makes sense. In fact I really do not know what is the best expressive-but-safe policy here. See the last point for a related issue
* what about clearing the full list of initializers (so no parameters in the new)?
  The annotation `noautoinit` will do the trick.
* what about setters?
  when one writes `class A var x: Int`, in fact the initializer is not the getter `x` but the setter `x=`. It is obvious once one understand that `var a = new A(10)` is just `var a = alloc A; a.x = 10; a.init`.
  However, I chose to implicitly try to search the setter first when given `x` as an element of the `autoinit` clause, so that one can write `autoinit x` instead of the less POLA `autoinit x=` (that also works by the way).
  Unfortunately this kind of heuristic makes the specification more complex, but also prevents one to use a method `x` as an initializer if a method `x=` exists.

Pull-Request: #1158
Reviewed-by: Lucas Bajolet <r4pass@hotmail.com>
Reviewed-by: Alexandre Terrasa <alexandre@moz-code.org>

17 files changed:
src/frontend/check_annotation.nit
src/model/model.nit
src/modelize/modelize_property.nit
tests/base_init_autoinit2.nit
tests/base_init_autoinit3.nit [new file with mode: 0644]
tests/base_init_noinit.nit
tests/sav/base_init_autoinit2_alt1.res
tests/sav/base_init_autoinit3.res [new file with mode: 0644]
tests/sav/base_init_autoinit3_alt1.res [new file with mode: 0644]
tests/sav/base_init_autoinit3_alt2.res [new file with mode: 0644]
tests/sav/base_init_autoinit3_alt4.res [new file with mode: 0644]
tests/sav/base_init_autoinit3_alt5.res [new file with mode: 0644]
tests/sav/base_init_autoinit3_alt6.res [new file with mode: 0644]
tests/sav/base_init_autoinit3_alt7.res [new file with mode: 0644]
tests/sav/base_init_autoinit3_alt8.res [new file with mode: 0644]
tests/sav/base_init_noinit_alt5.res
tests/sav/niti/base_init_autoinit3_alt1.res [new file with mode: 0644]

index a51bece..9408e95 100644 (file)
@@ -83,6 +83,7 @@ noinit
 readonly
 writable
 autoinit
+noautoinit
 cached
 nosuper
 old_style_init
index 00ca5cf..f0f4ff9 100644 (file)
@@ -1757,6 +1757,9 @@ abstract class MProperty
        # The visibility of the property
        var visibility: MVisibility
 
+       # Is the property usable as an initializer?
+       var is_autoinit = false is writable
+
        init
        do
                intro_mclassdef.intro_mproperties.add(self)
index 42dee1e..ba62b6e 100644 (file)
@@ -181,6 +181,7 @@ redef class ModelBuilder
                                        mparameters.add(mparameter)
                                end
                                initializers.add(npropdef.mpropdef.mproperty)
+                               npropdef.mpropdef.mproperty.is_autoinit = true
                        end
                        if npropdef isa AAttrPropdef then
                                if npropdef.mpropdef == null then return # Skip broken attribute
@@ -190,6 +191,7 @@ redef class ModelBuilder
                                        # For autoinit attributes, call the reader to force
                                        # the lazy initialization of the attribute.
                                        initializers.add(npropdef.mreadpropdef.mproperty)
+                                       npropdef.mreadpropdef.mproperty.is_autoinit = true
                                        continue
                                end
                                if npropdef.has_value then continue
@@ -202,9 +204,11 @@ redef class ModelBuilder
                                if msetter == null then
                                        # No setter, it is a old-style attribute, so just add it
                                        initializers.add(npropdef.mpropdef.mproperty)
+                                       npropdef.mpropdef.mproperty.is_autoinit = true
                                else
                                        # Add the setter to the list
                                        initializers.add(msetter.mproperty)
+                                       msetter.mproperty.is_autoinit = true
                                end
                        end
                end
@@ -218,39 +222,99 @@ redef class ModelBuilder
                        return
                end
 
-               # Search the longest-one and checks for conflict
-               var longest = spropdefs.first
-               if spropdefs.length > 1 then
-                       # Check for conflict in the order of initializers
-                       # Each initializer list must me a prefix of the longest list
-                       # part 1. find the longest list
-                       for spd in spropdefs do
-                               if spd.initializers.length > longest.initializers.length then longest = spd
+               # Look at the autoinit class-annotation
+               var autoinit = nclassdef.get_single_annotation("autoinit", self)
+               var noautoinit = nclassdef.get_single_annotation("noautoinit", self)
+               if autoinit != null then
+                       # Just throws the collected initializers
+                       mparameters.clear
+                       initializers.clear
+
+                       if noautoinit != null then
+                               error(autoinit, "Error: `autoinit` and `noautoinit` are incompatible.")
+                       end
+
+                       if autoinit.n_args.is_empty then
+                               error(autoinit, "Syntax error: `autoinit` expects method identifiers, use `noautoinit` to clear all autoinits.")
                        end
-                       # part 2. compare
-                       for spd in spropdefs do
-                               var i = 0
-                               for p in spd.initializers do
-                                       if p != longest.initializers[i] then
-                                               self.error(nclassdef, "Error: conflict for inherited inits {spd}({spd.initializers.join(", ")}) and {longest}({longest.initializers.join(", ")})")
-                                               return
+
+                       # Get and check each argument
+                       for narg in autoinit.n_args do
+                               var id = narg.as_id
+                               if id == null then
+                                       error(narg, "Syntax error: `autoinit` expects method identifiers.")
+                                       return
+                               end
+
+                               # Search the property.
+                               # To avoid bad surprises, try to get the setter first.
+                               var p = try_get_mproperty_by_name(narg, mclassdef, id + "=")
+                               if p == null then
+                                       p = try_get_mproperty_by_name(narg, mclassdef, id)
+                               end
+                               if p == null then
+                                       error(narg, "Error: unknown method `{id}`")
+                                       return
+                               end
+                               if not p.is_autoinit then
+                                       error(narg, "Error: `{p}` is not an autoinit method")
+                                       return
+                               end
+
+                               # Register the initializer and the parameters
+                               initializers.add(p)
+                               var pd = p.intro
+                               if pd isa MMethodDef then
+                                       # Get the signature resolved for the current receiver
+                                       var sig = pd.msignature.resolve_for(mclassdef.mclass.mclass_type, mclassdef.bound_mtype, mclassdef.mmodule, false)
+                                       mparameters.add_all sig.mparameters
+                               else
+                                       # TODO attributes?
+                                       abort
+                               end
+                       end
+               else if noautoinit != null then
+                       if initializers.is_empty then
+                               warning(noautoinit, "useless-noautoinit", "Warning: the list of autoinit is already empty.")
+                       end
+                       # Just clear initializers
+                       mparameters.clear
+                       initializers.clear
+               else
+                       # Search the longest-one and checks for conflict
+                       var longest = spropdefs.first
+                       if spropdefs.length > 1 then
+                               # Check for conflict in the order of initializers
+                               # Each initializer list must me a prefix of the longest list
+                               # part 1. find the longest list
+                               for spd in spropdefs do
+                                       if spd.initializers.length > longest.initializers.length then longest = spd
+                               end
+                               # part 2. compare
+                               for spd in spropdefs do
+                                       var i = 0
+                                       for p in spd.initializers do
+                                               if p != longest.initializers[i] then
+                                                       self.error(nclassdef, "Error: conflict for inherited inits {spd}({spd.initializers.join(", ")}) and {longest}({longest.initializers.join(", ")})")
+                                                       return
+                                               end
+                                               i += 1
                                        end
-                                       i += 1
                                end
                        end
-               end
 
-               # Can we just inherit?
-               if spropdefs.length == 1 and mparameters.is_empty and defined_init == null then
-                       self.toolcontext.info("{mclassdef} inherits the basic constructor {longest}", 3)
-                       mclassdef.mclass.root_init = longest
-                       return
-               end
+                       # Can we just inherit?
+                       if spropdefs.length == 1 and mparameters.is_empty and defined_init == null then
+                               self.toolcontext.info("{mclassdef} inherits the basic constructor {longest}", 3)
+                               mclassdef.mclass.root_init = longest
+                               return
+                       end
 
-               # Combine the inherited list to what is collected
-               if longest.initializers.length > 0 then
-                       mparameters.prepend longest.new_msignature.mparameters
-                       initializers.prepend longest.initializers
+                       # Combine the inherited list to what is collected
+                       if longest.initializers.length > 0 then
+                               mparameters.prepend longest.new_msignature.mparameters
+                               initializers.prepend longest.initializers
+                       end
                end
 
                # If we already have a basic init definition, then setup its initializers
@@ -928,10 +992,11 @@ redef class AAttrPropdef
                has_value = n_expr != null or n_block != null
 
                var atnoinit = self.get_single_annotation("noinit", modelbuilder)
+               if atnoinit == null then atnoinit = self.get_single_annotation("noautoinit", modelbuilder)
                if atnoinit != null then
                        noinit = true
                        if has_value then
-                               modelbuilder.error(atnoinit, "Error: `noinit` attributes cannot have an initial value")
+                               modelbuilder.error(atnoinit, "Error: `noautoinit` attributes cannot have an initial value")
                                return
                        end
                end
index afce0f9..5930588 100644 (file)
@@ -16,9 +16,9 @@ import kernel
 
 class A
        var a: Object = get(5) is lazy
-       var b: Object is noinit
-       #alt1#var b2: Object = get(-4) is noinit
-       var c: Object is noinit
+       var b: Object is noautoinit
+       #alt1#var b2: Object = get(-4) is noautoinit
+       var c: Object is noautoinit
        var d: Object = get(2) is autoinit
        #alt2#var d2: Object = get(-2) is autoinit, lazy
        var e: Object = get(1)
diff --git a/tests/base_init_autoinit3.nit b/tests/base_init_autoinit3.nit
new file mode 100644 (file)
index 0000000..37c2cb2
--- /dev/null
@@ -0,0 +1,67 @@
+# This file is part of NIT ( http://www.nitlanguage.org ).
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import kernel
+
+class A
+       var i: Int
+end
+
+class B
+       autoinit b, i #alt2# autoinit foo #alt4# autoinit fail
+       super A
+       var b: Bool #alt5# var b: Bool is noinit
+       fun foo do end
+end
+
+class C
+       super A
+       var f: Float
+end
+
+class D
+       autoinit i, b, f
+       super B
+       super C
+end
+
+class E
+       noautoinit #alt6#
+       #alt6,7# autoinit
+       super A
+       var a: A
+end
+
+class F
+       #alt8#noautoinit
+end
+
+var a = new A(1)
+a.i.output
+
+var b = new B(false, 2)
+b.i.output
+b.b.output
+
+var c = new C(3, 3.3)
+c.i.output
+c.f.output
+
+var d = new D(4, true, 4.4)
+d.i.output
+d.b.output
+d.f.output
+
+var e = new E
+#alt1# e.a.i.output
index 87de46e..f129d53 100644 (file)
@@ -15,8 +15,8 @@
 import kernel
 
 class A
-       var x: Object is noinit #alt1,3# var x: Object
-       var y: Object is noinit #alt2,3# var y: Object
+       var x: Object is noautoinit #alt1,3# var x: Object
+       var y: Object is noautoinit #alt2,3# var y: Object
        fun work
        do
                if isset _x then x.output
index 9c9fbe0..25528d8 100644 (file)
@@ -1 +1 @@
-alt/base_init_autoinit2_alt1.nit:20,30--35: Error: `noinit` attributes cannot have an initial value
+alt/base_init_autoinit2_alt1.nit:20,30--39: Error: `noautoinit` attributes cannot have an initial value
diff --git a/tests/sav/base_init_autoinit3.res b/tests/sav/base_init_autoinit3.res
new file mode 100644 (file)
index 0000000..6851c6d
--- /dev/null
@@ -0,0 +1,8 @@
+1
+2
+false
+3
+3.300000
+4
+true
+4.400000
diff --git a/tests/sav/base_init_autoinit3_alt1.res b/tests/sav/base_init_autoinit3_alt1.res
new file mode 100644 (file)
index 0000000..92263fd
--- /dev/null
@@ -0,0 +1,9 @@
+Runtime error: Uninitialized attribute _a (alt/base_init_autoinit3_alt1.nit:43)
+1
+2
+false
+3
+3.300000
+4
+true
+4.400000
diff --git a/tests/sav/base_init_autoinit3_alt2.res b/tests/sav/base_init_autoinit3_alt2.res
new file mode 100644 (file)
index 0000000..cd95aed
--- /dev/null
@@ -0,0 +1 @@
+alt/base_init_autoinit3_alt2.nit:22,11--13: Error: `foo` is not an autoinit method
diff --git a/tests/sav/base_init_autoinit3_alt4.res b/tests/sav/base_init_autoinit3_alt4.res
new file mode 100644 (file)
index 0000000..269b2fd
--- /dev/null
@@ -0,0 +1 @@
+alt/base_init_autoinit3_alt4.nit:22,11--14: Error: unknown method `fail`
diff --git a/tests/sav/base_init_autoinit3_alt5.res b/tests/sav/base_init_autoinit3_alt5.res
new file mode 100644 (file)
index 0000000..67f272a
--- /dev/null
@@ -0,0 +1,2 @@
+alt/base_init_autoinit3_alt5.nit:22,11: Error: `b=` is not an autoinit method
+alt/base_init_autoinit3_alt5.nit:34,14: Error: `b=` is not an autoinit method
diff --git a/tests/sav/base_init_autoinit3_alt6.res b/tests/sav/base_init_autoinit3_alt6.res
new file mode 100644 (file)
index 0000000..e95f861
--- /dev/null
@@ -0,0 +1 @@
+alt/base_init_autoinit3_alt6.nit:40,2--41,9: Syntax error: `autoinit` expects method identifiers, use `noautoinit` to clear all autoinits.
diff --git a/tests/sav/base_init_autoinit3_alt7.res b/tests/sav/base_init_autoinit3_alt7.res
new file mode 100644 (file)
index 0000000..9fb22d1
--- /dev/null
@@ -0,0 +1,2 @@
+alt/base_init_autoinit3_alt7.nit:41,2--9: Error: `autoinit` and `noautoinit` are incompatible.
+alt/base_init_autoinit3_alt7.nit:41,2--9: Syntax error: `autoinit` expects method identifiers, use `noautoinit` to clear all autoinits.
diff --git a/tests/sav/base_init_autoinit3_alt8.res b/tests/sav/base_init_autoinit3_alt8.res
new file mode 100644 (file)
index 0000000..f88f100
--- /dev/null
@@ -0,0 +1,9 @@
+alt/base_init_autoinit3_alt8.nit:47,2--11: Warning: the list of autoinit is already empty.
+1
+2
+false
+3
+3.300000
+4
+true
+4.400000
index b79ac75..1f0479d 100644 (file)
@@ -1 +1 @@
-alt/base_init_noinit_alt5.nit:26,23--28: Error: `noinit` attributes cannot have an initial value
+alt/base_init_noinit_alt5.nit:26,23--28: Error: `noautoinit` attributes cannot have an initial value
diff --git a/tests/sav/niti/base_init_autoinit3_alt1.res b/tests/sav/niti/base_init_autoinit3_alt1.res
new file mode 100644 (file)
index 0000000..70e53f3
--- /dev/null
@@ -0,0 +1,9 @@
+Runtime error: Uninitialized attribute _a (alt/base_init_autoinit3_alt1.nit:67)
+1
+2
+false
+3
+3.300000
+4
+true
+4.400000