This causes Geary to break when switching to gir--all the properties become owned get, which doesn't match the implementation.

Log for geary: http://paldo.org:8010/builders/vala-staging/builds/17/steps/geary/logs/stdio

The problem occurs for Gtk.Scrollable.vadjustment and hadjustment.  The definition for Gtk.Scrollable:

	[CCode (cheader_filename = "gtk/gtk.h", type_cname = "GtkScrollableInterface", type_id = "gtk_scrollable_get_type ()")]
	public interface Scrollable : GLib.Object {
		public unowned Gtk.Adjustment get_hadjustment ();
		public Gtk.ScrollablePolicy get_hscroll_policy ();
		public unowned Gtk.Adjustment get_vadjustment ();
		public Gtk.ScrollablePolicy get_vscroll_policy ();
		public void set_hadjustment (Gtk.Adjustment? hadjustment);
		public void set_hscroll_policy (Gtk.ScrollablePolicy policy);
		public void set_vadjustment (Gtk.Adjustment? vadjustment);
		public void set_vscroll_policy (Gtk.ScrollablePolicy policy);
		[NoAccessorMethod]
		public abstract Gtk.Adjustment hadjustment { owned get; set construct; }
		[NoAccessorMethod]
		public abstract Gtk.ScrollablePolicy hscroll_policy { get; set; }
		[NoAccessorMethod]
		public abstract Gtk.Adjustment vadjustment { owned get; set construct; }
		[NoAccessorMethod]
		public abstract Gtk.ScrollablePolicy vscroll_policy { get; set; }
	}

The general problem is:
1) The property is abstract
2) The accessor method is not
When overriding the property, vala overrides the accessor. Therefore:
1) Either keep getter and setter methods
2) Or tell vala to override the property without overriding the accessor, but use the accessor when accessing the property.

Now for interfaces, maybe it's not possible at all to have an overridable accessor? Is there any example of interface with abstract property and abstract accessor anywhere?

IIRC if we make both the property and method overridable then Vala will force you to implement both, which will result in a C error, so we can't do both.

I think 3 is the right way to go, so we would end up with

    public interface Scrollable : GLib.Object {
        public unowned Gtk.Adjustment get_hadjustment ();
        public void set_vadjustment (Gtk.Adjustment? vadjustment);
        public abstract Gtk.Adjustment hadjustment { get; set construct; }
    }

I don't know if it's causing problems, but set_vadjustment is nullable--If that's the problem I think we should just ignore nullability for now.  I'm planning on putting together a patch to the gir parser that would mark the property as nullable if both the getter and the setter are, and maybe emit a warning if there is a mismatch (though I'll probably keep that part local and just add the annotations upstream where appropriate, or at least hide it behind a flag).

I wasn't clear, when I said overridable I meant overridable in C. That is, the problem is that the accessors don't have an entry in the vtable.
By overriding a property in vala, the codegen also overrides the accessors at C level. But the gtk gir for filechooser provide accessors that are not overridable in C.
Your proposal, without changes to the codegen, can't work, because vala expects virtual accessors from hadjustment property, which is not the case. If you try inheriting, it fails to compile.

Okay, so then the problem is that the properties are marked as abstract when they shouldn't be.  That would match the current VAPI, too.

No, the gir is correct:
accessors -> not virtual
property -> virtual
Conclusion: property does not have accessors, because otherwise vala would override accessors at C level.
Therefore also vala is correct.
Hence my question, again: is there an interface that has both property and accessors virtual? If not, we can avoid adding some other attribute like [CCode (virtual_accessors = false)].

How could both the accessors and the property be virtual?  Wouldn't they try to use the same vfunc slot?  I can't say there aren't any, but I can't think of any, and if they do exist I can't see how it would work.

My concern here is backwards compatibility.  https://git.gnome.org/browse/geary/tree/src/client/composer/scrollable-overlay.vala#n11 breaks right now, as does a lot of other code.

Ok, vala itself makes both the gobject property and the accessors abstract/virtual. So there's a need for [CCode (virtual_accessors = false)] on top of properties, or [NoAccessorMethod (something_else here)].

The old gtk bindings don't make hadjustment abstract.
I'm a little lost here on what does it mean for an interface property to not be abstract.

Ah of course a non-abstract property is simply a pair of methods with body, without a gobject property in vala.

So we were exploiting the fact that gtk library does not provide vfunc for accessors, *but* it does register a gobject property.

The obvious fix in the gir parser is:
(it's an interface) and (we found accessors) and (accessors are not virtual) -> prop.is_abstract = false.

No that solution does not work. Interface properties must always be abstract. In C however many provide non-overridable accessors.
[NoAccessorMethod] with { get; set; } is just bad because it leaks.
This is something that should have gone in the syntax, like abstract Object foo { concrete get; set; }
That's because it should really have been Object foo { abstract get; abstract set; } since the beginning :(

An alternative is to force the implementation of properties, regardless of whether they are abstract or not.

Recap:
1) [NoAccessorMethod] Object foo { get; set; } leaks on the getter
2) Object foo { get; set; } currently in interfaces means it's just a pair of get/set methods. In fact vala does not register a gobject property if you provide a body for get and set.
3) abstract Object foo { get; set; } requires both getter and setter to be overridable

Problem:
- Most C interfaces have non-overridable getter/setter that simply g_object_get/set the gobject property. This means Vala classes implementing the interface must implement the gobject property but they must not override the getter/setter.
- Existing code is implementing properties as Object foo { get; set; } that are [NoAccessorMethod] abstract Object foo { get; set; } in interfaces.

Solutions:
1) Just break backward compatibility. C interface getter/setter are simply doing g_object_get/set, which is what vala already does with [NoAccessorMethod].
2) abstract Object foo { concrete get; concrete set; } which tells Vala classes to implement the gobject property, but not override the accessors.

(In reply to comment #12)
> Solutions:
> 1) Just break backward compatibility. C interface getter/setter are simply
> doing g_object_get/set, which is what vala already does with
> [NoAccessorMethod].

I don't think this is really an option.  I have a feeling it would break a pretty large number of applications--it's seems like a pretty common pattern, at least in GTK+ (judging by the diff between the gidl and gir bindings).

> 2) abstract Object foo { concrete get; concrete set; } which tells Vala classes
> to implement the gobject property, but not override the accessors.

I'm not very fond of this, either...  I understand your reasoning, but it's a pretty arcane detail.  I think I would prefer something like [ConcreteAccessorMethod] or [CCode (concrete_accessor_method = true)].

Can't be CCode, but [ConcreteAccessorMethod] seems fine.

commit b43037bb56875715baed9686d31df61a8581d23b
Author: Luca Bruno <lucabru@src.gnome.org>
Date:   Mon Dec 22 12:00:17 2014 +0100

    Support concrete accessors for abstract properties in bindings
    
    Fixes bug 730744

Ready to revert if anything goes completely wrong. We can find another solution.


This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.

Regenerated the bindings in:

commit 525cd13798aa16c0869d40c5f5a7593a22eb73f9
Author: Luca Bruno <lucabru@src.gnome.org>
Date:   Mon Dec 22 12:05:46 2014 +0100

    Regenerate all the bindings with ConcreteAccessor