From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15899 invoked by alias); 23 Apr 2014 20:06:30 -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 15844 invoked by uid 89); 23 Apr 2014 20:06:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 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-vc0-f174.google.com Received: from mail-vc0-f174.google.com (HELO mail-vc0-f174.google.com) (209.85.220.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 23 Apr 2014 20:06:28 +0000 Received: by mail-vc0-f174.google.com with SMTP id ld13so1760371vcb.5 for ; Wed, 23 Apr 2014 13:06:25 -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=IpdcV1ZhE1TRfQWsa9vGd8LmuUT/ZJ24EDYfZ0ChcuQ=; b=YT4NLrLKuisn4AqzjrEdq4U9Vguli/MSVsuRXkJqdkuPvMqJ4TQ/PocaR2fK/mug6Q PqCdsXEx4RldwF+LpK8Yn7b0O4BBvP8wUJEb48zIyphENAK4x/06clth11rP2N0KK72T yLwDn6mVOgV5vAm0K+ssLVulUXlLrHN6kChsFaRfAQLLMY4oYLHVFyXiOdKDtGdiC+CD xqzsbBPy/Sdd+OjpcgcOXntN/O/yGIVe+6zNeCXtb9WPY7GAVxxrK6iW8BQpMnyaivhm t1We4gzhSBu24b9jWV48sHbhbsBL3Px7VH9CTc67C3aqRHGgxFEvToViVM9mLrzjPN6P jYtA== X-Gm-Message-State: ALoCoQm0OclKnt7yq9Z1gylkV/Ev0kAAOpuA2UQnVYNAmFubCCs9T9l7+mkqm7JHrmtXvzYYsVg6EbZneamnKYItFGD0jm3OPc8rIMI4cwxZA1uNHRk8M6RZ6DZ0g/rwaDaQJV3uYvTGm7EfRWhqamTXV8phb3hsXOutyWWI2OBtjkZGR9YQlK/KVmjzOe1rk3+14H2TS+56XRf8WG3jkWJo3IjjcWIMCw== MIME-Version: 1.0 X-Received: by 10.220.95.204 with SMTP id e12mr1140013vcn.37.1398283585572; Wed, 23 Apr 2014 13:06:25 -0700 (PDT) Received: by 10.52.13.101 with HTTP; Wed, 23 Apr 2014 13:06:25 -0700 (PDT) In-Reply-To: References: <21333.45108.381336.447391@ruffy.mtv.corp.google.com> Date: Wed, 23 Apr 2014 20:06:00 -0000 Message-ID: Subject: Re: [PATCH v14 3/4] Add support for lookup, overload resolution and invocation of C++ debug methods From: Doug Evans To: Siva Chandra Cc: gdb-patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-04/txt/msg00468.txt.bz2 On Tue, Apr 22, 2014 at 1:46 PM, Siva Chandra wrote: > 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. A good question we need to answer is: What happens if a user does "ptype" on an expression involving a debug method? [ptype uses evaluate_type which is an example of using EVAL_AVOID_SIDE_EFFECTS] A user should be able to do that and have confidence that inferior state will not be modified. Reading the vtable is ok, but if the debug method is implementing, say, some kind of iterator then modifying inferior state would not be ok. I tried "ptype a1 + a2" from your py-debugmethods.exp testcase, and the debug method does get invoked. Is that ok? In many cases probably, but it doesn't seem likely to be ok in the general case. How to achieve that is another good question. [Ideally we wouldn't impose EVAL_AVOID_SIDE_EFFECTS on debug method writers.] I have been assuming that debug methods could fit in with the existing logic and the solution would just fall out. That needs to be verified of course. :-) >> 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? I was thinking of the invoke_debug_method call in evaluate_subexp_standard. There's a lot of code for case OP_FUNCALL, and the call to call_function_by_hand appears at the very bottom of the case. IWBN if the actual call to the debug method was done there too. >> 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? The names are a bit confusing alright. I left TYPE_CODE_INTERNAL_FUNCTION alone because it is used for $foo(bar). Convenience functions have their own syntax. Maybe we *could* morph TYPE_CODE_INTERNAL_FUNCTION into something more general purpose, I'm not sure. > 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). Yeah. Though once we have support for static methods will they still be "debug methods"? I'd like to avoid pinning down the term "debug methods" to a specific use-case (c++ non-static methods). [assuming that is the name we're sticking with for this feature] > 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. It's mostly just a matter of naming, but the term "Convenience Functions" already has a specific connotation that implies a leading $ in the name. Extending convenience functions to foo() in addition to $foo() could also inflict confusion. It would be odd, I think, to the user to have non-static methods be "debug methods" and static methods be "convenience functions". I don't have a strong opinion though. IWBN to hear from others in the community but we may be on our own on this one. btw, "making debug methods less special case" was more aimed at the implementation (IOW treating them internally no different than other functions: TYPE_CODE_FUNC and TYPE_CODE_INTERNAL_FUNCTION). > 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. To the person reading gdb code, debug methods are conceptually no different than TYPE_CODE_INTERNAL_FUNCTION when it comes to actually invoking them. Yeah, they're looked up differently, but once they're looked up they're just some "external" code that gdb will invoke to evaluate an expression the user entered. Thus my mental model of how I expect to see debug methods implemented in the code should match to the extent possible and reasonable how I see TYPE_CODE_INTERNAL_FUNCTION implemented. Where it doesn't it means more mental effort has to go in to understand and maintain them, and more state to have to keep in cache while working with the code.