Received: by 10.224.47.145 with SMTP id n17mr297305qaf.6.1298394067273;
        Tue, 22 Feb 2011 09:01:07 -0800 (PST)
X-BeenThere: protobuf@googlegroups.com
Received: by 10.224.3.230 with SMTP id 38ls26674qao.0.p; Tue, 22 Feb 2011
 09:01:03 -0800 (PST)
Received: by 10.224.36.200 with SMTP id u8mr289034qad.7.1298394062920;
        Tue, 22 Feb 2011 09:01:02 -0800 (PST)
MIME-Version: 1.0
Received: by 10.224.201.74 with SMTP id ez10mr129513qab.15.1298379241658; Tue,
 22 Feb 2011 04:54:01 -0800 (PST)
Received: by v16g2000vbq.googlegroups.com with HTTP; Tue, 22 Feb 2011 04:54:01
 -0800 (PST)
Date: Tue, 22 Feb 2011 04:54:01 -0800 (PST)
X-IP: 170.148.215.156
User-Agent: G2/1.0
X-HTTP-UserAgent: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1;
 Trident/4.0; .NET CLR 1.0.3705; .NET CLR 1.1.4322; .NET CLR 2.0.50727; .NET
 CLR 3.0.4506.2152; .NET CLR 3.5.30729; MS-RTC LM 8),gzip(gfe)
Message-ID: <762165e0-fe3c-4b03-a6d3-41e6286d27ca@v16g2000vbq.googlegroups.com>
Subject: Suspected bug in java 2.4.0a Sub-builders
From: jamesm <james.mul...@gmail.com>
To: Protocol Buffers <protobuf@googlegroups.com>
Content-Type: text/plain; charset=ISO-8859-1

I have found two things that look to me like bugs in sub-builder
handling; one is in the generated code; the second is in the base
class GeneratedMessage.

Consider the following:

message Inner {
 optional int32 somefield = 1;
}

message Outer{
  optional Inner  inner = 1;
}

Generated code for sub builder is roughtly as follows:
      public Inner.Builder getInnerBuilder() {
        bitField0_ |= 0x00000004; //wrong - we've not changed it yet
        onChanged(); //wrong - we've not changed it yset
        return getInnerFieldBuilder().getBuilder();
      }

The net effect is that calling:
outer.getInnerBuilder() marks outer as dirty; which in my view is
incorrect

The second issue is in GeneratedMessage.Builder:

    protected final void onChanged() {
      if (isClean && builderParent != null) {
        builderParent.markDirty();

        // Don't keep dispatching invalidations until build is called
again.
        isClean = false;
      }
    }
This again seems to be incorrect; it should be as follows:
    protected final void onChanged() {
      if (isClean && builderParent != null) {
        builderParent.markDirty();
      }
        // Don't keep dispatching invalidations until build is called
again.
        isClean = false;
    }

I can see how these two bugs together would pass a basic Junit; but
they showed up in some more extensive tests that are part of the
system that I'm building.

With these changes made we would have the desired effect that only
when a modification is made to the sub builder does that builder (and
it's parent) get marked as dirty.

Please can you confirm that the above are bugs and whether they can be
fixed before 2.4.0 final?

Thanks