From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27630 invoked by alias); 22 Apr 2014 20:46:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 27621 invoked by uid 89); 22 Apr 2014 20:46:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-wg0-f51.google.com Received: from mail-wg0-f51.google.com (HELO mail-wg0-f51.google.com) (74.125.82.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Tue, 22 Apr 2014 20:46:56 +0000 Received: by mail-wg0-f51.google.com with SMTP id k14so11968wgh.34 for ; Tue, 22 Apr 2014 13:46:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=UkT6wQpVVRejhlXr1bOsKTPuUjWr7JBKb8OjtKvDZ4Q=; b=h+G10P7zPW++yfZb/k/C3zVrWDKDTFG5Ji4rELs8xHNM/vKQbTZtaPSvtRLJTdSEsg X4tS4DDL4NcVvI8GOGoskcjoyGIriOWCSm2HTvOejj8sm6ipug4IdA/LlTJ7Oo9RzMs4 pjeQIy3LmcmwF4eEdant6GR85lh1ejax/O9YLmmCUJj5hOiIy2ce808fcAzkV3Z4IybX aPQFlvdwgOd7Josn4JnjyQabr/JFIc9WLypW4EGKLr0pGDf5zR2QvzOcH1dPgY9CIKs6 3xELkZMINkfSgSHQtrNGAOkJAlZadV4fysyPJL8iHQ9Vhygr0E//QpOF9UM9ksqIKd90 QFMg== X-Gm-Message-State: ALoCoQnAg5iVpZlD/zwHsoXY8JY7ALSYSIFAUN3POVnxrMt+BcbIv0OfiTu6jSMq/7fGV9aUb4dXxA1Iq1r0JuOQgbS2KEk+6OPKTDi6gQykJqemSQLvLPmo16dk/0tGyIUNCnv4zhBVFkKLuqKd0+JCAJlTCCiMOKWGUUPFuvY+YWZVUQJMpVzyLIw+6Oq2PIROL0uggnHpLFGnyJSFatI+jvk3qPA9FA== MIME-Version: 1.0 X-Received: by 10.180.206.36 with SMTP id ll4mr291266wic.57.1398199612634; Tue, 22 Apr 2014 13:46:52 -0700 (PDT) Received: by 10.217.51.7 with HTTP; Tue, 22 Apr 2014 13:46:52 -0700 (PDT) In-Reply-To: <21333.45108.381336.447391@ruffy.mtv.corp.google.com> References: <21333.45108.381336.447391@ruffy.mtv.corp.google.com> Date: Tue, 22 Apr 2014 20:46:00 -0000 Message-ID: Subject: Re: [PATCH v14 3/4] Add support for lookup, overload resolution and invocation of C++ debug methods From: Siva Chandra To: Doug Evans Cc: gdb-patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-04/txt/msg00439.txt.bz2 On Mon, Apr 21, 2014 at 4:56 PM, Doug Evans wrote: > Another issue I have is this bit of code in, e.g., value_x_binop: > > value_user_defined_op (&arg1, argvec + 1, tstr, &static_memfuncp, 2, > &argvec[0], &dm_worker); > > if (argvec[0]) > { > if (static_memfuncp) > { > argvec[1] = argvec[0]; > argvec++; > } > if (noside == EVAL_AVOID_SIDE_EFFECTS) > { > struct type *return_type; > > return_type > = TYPE_TARGET_TYPE (check_typedef (value_type (argvec[0]))); > return value_zero (return_type, VALUE_LVAL (arg1)); > } > return call_function_by_hand (argvec[0], 2 - static_memfuncp, > argvec + 1); > } > if (dm_worker != NULL) > { > struct cleanup *dm_worker_cleanup = make_cleanup (xfree, dm_worker); > struct value *ret_val = invoke_debug_method (dm_worker, arg1, &arg2, 1); > > do_cleanups (dm_worker_cleanup); > return ret_val; > } > > Does this mean we call the debug method (assuming it wins) for the > case of noside == EVAL_AVOID_SIDE_EFFECTS? To be frank, I never understood why this option exists at all. If side effects (like even reading target memory) are not desired, why evaluate expressions at all? And, fwiw, there is a piece of code in find_oload_match which looks up the most derived type of an object which involves reading the vtable much before all this. > Here and elsewhere there's logic that the debug method code is not > being included in, and it makes me want to do things differently. Can you point me to an example different from EVAL_AVOID_SIDE_EFFECTS? > Fortunately, I think (though I haven't implemented it) it won't be hard. > Just like we have TYPE_CODE_INTERNAL_FUNCTION we could also have > TYPE_CODE_EXTERNAL_FUNCTION, and use that to make debug methods less > special case. > clone_debug_method_worker could return a struct value that is a > TYPE_CODE_EXTERNAL_FUNCTION, and then just before the call to > call_function_by_hand that would happen for normal methods, > we'd have a check for TYPE_CODE_EXTERNAL_FUNCTION and call > call_debug_method instead. I am putting down my notes and questions. 1. I think TYPE_CODE_INTERNAL_FUNCTIONs can also be implemented in extension languages. So, what makes them internal and different from TYPE_CODE_EXTERNAL_FUNCTION? I have tried to convince myself that the name "INTERNAL" exists because of historical reasons and does not mean anything today. Hence, is TYPE_CODE_EXTERNAL_FUNCTION a good name? Am I missing something? 2.If debug methods are to be represented by a value (say, of type TYPE_CODE_DEBUG_METHOD), then a new value should be created for every debug method invocation. I do not know if this is good or bad, and I do not know if this happens for internal functions as well. A fundamental difference (atleast in my mind) is that internal functions have a name by definition (at least currently) and are invoked by that name. Debug methods do not have a name by definition that they are invoked with. In fact, debug methods have different invocation names based on the context (ie. template arguments). 3. Extending debug methods to functions (if that is what you meant by making debug methods less special case); I think this would lead to a lot of confusion between convenience functions, debug methods and "debug functions". Best would be to support only two mutually exclusive features "Debug Methods" and "Convenience Functions" with latter extended to have the ability to replace existing source functions. This is my personal opinion which I am ready to change if you prescribe a specific different way. 4. What benefit, apart from lesser number of arguments to find_oload_match, do you see? I guess this is related to the above question on what else is the debug methods logic currently missing. Thanks, Siva Chandra