From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17268 invoked by alias); 16 Aug 2004 19:07:55 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 17255 invoked from network); 16 Aug 2004 19:07:53 -0000 Received: from unknown (HELO mail-out4.apple.com) (17.254.13.23) by sourceware.org with SMTP; 16 Aug 2004 19:07:53 -0000 Received: from mailgate2.apple.com (a17-128-100-204.apple.com [17.128.100.204]) by mail-out4.apple.com (8.12.11/8.12.11) with ESMTP id i7GJAbW4007206 for ; Mon, 16 Aug 2004 12:10:37 -0700 (PDT) Received: from relay1.apple.com (relay1.apple.com) by mailgate2.apple.com (Content Technologies SMTPRS 4.3.6) with ESMTP id ; Mon, 16 Aug 2004 12:07:53 -0700 Received: from [17.201.24.57] (polskifiat.apple.com [17.201.24.57]) by relay1.apple.com (8.12.11/8.12.11) with ESMTP id i7GJ7oam016168; Mon, 16 Aug 2004 12:07:51 -0700 (PDT) In-Reply-To: References: Mime-Version: 1.0 (Apple Message framework v619) Content-Type: text/plain; charset=US-ASCII; format=flowed Message-Id: <918394DF-EFB7-11D8-8323-000393673036@apple.com> Content-Transfer-Encoding: 7bit Cc: GCC Patches From: Ziemowit Laski Subject: Re: Date: Mon, 16 Aug 2004 19:28:00 -0000 To: Mark Mitchell , Zack Weinberg X-SW-Source: 2004-08/txt/msg01097.txt.bz2 Mark, Zack, Please see my responses below. > > This is by far the biggest chunk of the ObjC++ integration work; its > > purpose is to set the stage > > for ObjC++ proper. > > > > I've broken down the patch into two parts, A and B. Part A (this > > one) touches common parts of the compiler, and therefore requires > > global approval. > > You need to explain, in detail, what each change does and why. > > You have broken up this patch along the wrong lines. Never break a > patch into pieces which could not be committed independently. Changes The reason I broke the patch up is simply to ease the review process, since I can take maintainer responsibility for a large chunk of it. I apologize if this is confusing; this _is_ really all one giant patch. > which are strictly mechanical - e.g. renaming functions - should > always be submitted separately from other changes. Prototypes for new > functions should always be submitted alongside the new functions > themselves. In this case it probably makes sense for you to send a > patch consisting solely of mechanical changes; a patch consisting > solely of addition of new functions (it's okay to add new functions > before the code that uses them); a patch consisting of miscellaneous > changes to common code, *along with* the minimal set of associated > changes to the Objective C front end; and finally the remainder of the > changes to the Objective C front end. You are expected to test, > submit for approval, and (if approved) commit each of these patches > independently. This is the part that I find problematic. :-( The work contained in the two patches I posted last night, in addition to a couple of patches I committed previously (and a few more I have yet to offer) is all part of my ObjC++ integration (approved by the Steering Committee for 3.5 integration). Furthermore, these bits already live in objc-improvements-branch, available for the bootstrapping pleasure of all. Therefore, I would kindly request that my ongoing work be treated as a branch integration project. Breaking this patch up into numerous tiny pieces that when put together will reconstitute the original patch does not offer any benefits that I can think of, and does in fact have two major drawbacks: (1) it takes a lot more time and (2) it is more susceptible to pilot error due to its fragmented nature. Mark, it would be very helpful if you could opine as to how I should proceed. Of course, all comments and criticisms (such as those below) are more than welcome, and I shall address them. I'd just just that I'd much prefer to (continue to) receive such comments regarding the patches as I posted them. :-) Now on to Zack's nits: > > +/****** OBJECTIVE-C / OBJECTIVE-C++ ENTRY POINTS ******/ > > No SCREAMING, no extra asterisks: > > /* Objective-C and/or Objective-C++ entry points. */ Ok; didn't mean to yell at anyone. :-) > > > +/* The following ObjC/ObjC++ functions are called by the C and/or > C++ > > + front-ends; they all must have corresponding stubs in > stub-objc.c. > > */ > > Your mailer is still word-wrapping patches. Fix it. Hm... it looks fine from here; perhaps I shall explicitly attach patches instead of pasting them into the window from now on. :-) > > > @@ -6662,6 +6664,12 @@ build_cdtor (int method_type, tree cdtor > > { > > tree body = 0; > > > > + /* The Objective-C metadata initializer (if any) must be run > > + _before_ all other static constructors. */ > > + if (c_dialect_objc () && (method_type == 'I') > > + && objc_static_init_needed_p ()) > > + cdtors = objc_generate_static_init_call (cdtors); > > + > > if (!cdtors) > > return; > > This chunk of logic belongs in the caller of build_cdtor. It can be > expressed much more cleanly there. Ok; I'll take a look. > > - if (!objc_is_public (datum, component)) > > + if (c_dialect_objc () && !objc_is_public (datum, component)) > > Most objc_* functions are designed not to need guarding with > c_dialect_objc(), therefore I think it best if all of them don't; in > other words, please cause objc_is_public to always return true when > !c_dialect_objc, so this change will be unnecessary. Again, perhaps Mark can issue a ruling here. I thought that c_dialect_objc() should be used because (1) it offers a clear demarcation point (i.e., "this is ObjC-specific functionality") and (2) it improves performance (checking a bit is a lot quicker than calling a function). > > > - {".i", "@cpp-output", 0, 1, 0}, > > - {"@cpp-output", > > + {".i", "@c-cpp-output", 0, 1, 0}, > > + {"@c-cpp-output", > > This is a gratuitous change to the user interface which may break > scripts. You may not make this change. (You may add -x c-cpp-output, > but -x cpp-output must continue to have its existing meaning.) Yes, you're right. Truth be told, this is the one piece I actually _should_ yank out my patch, since it is a logically separate animal. Thanks, --Zem -------------------------------------------------------------- Ziemowit Laski 1 Infinite Loop, MS 301-2K Mac OS X Compiler Group Cupertino, CA USA 95014-2083 Apple Computer, Inc. +1.408.974.6229 Fax .5477