From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32684 invoked by alias); 8 Aug 2017 21:28:00 -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 32652 invoked by uid 89); 8 Aug 2017 21:27:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.8 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_STOCKGEN,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=1774 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 08 Aug 2017 21:27:58 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 192FF76A1E for ; Tue, 8 Aug 2017 21:27:57 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 192FF76A1E Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=keiths@redhat.com Received: from valrhona.uglyboxes.com (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B4BDD6C51A; Tue, 8 Aug 2017 21:27:56 +0000 (UTC) Subject: Re: [PATCH 31/40] Handle custom completion match prefix / LCD To: Pedro Alves , gdb-patches@sourceware.org References: <1496406158-12663-1-git-send-email-palves@redhat.com> <1496406158-12663-32-git-send-email-palves@redhat.com> From: Keith Seitz Message-ID: Date: Tue, 08 Aug 2017 21:28:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1496406158-12663-32-git-send-email-palves@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-08/txt/msg00159.txt.bz2 On 06/02/2017 05:22 AM, Pedro Alves wrote: > diff --git a/gdb/completer.h b/gdb/completer.h > index df90822..db781ab 100644 > --- a/gdb/completer.h > +++ b/gdb/completer.h > @@ -116,12 +116,44 @@ private: > std::string m_storage; > }; > > +/* The result of a successful completion match, but for least common > + denominator (LCD) computation. Some completers provide matches > + that don't start with the completion "word". E.g., completing on > + "b push_ba" on a C++ program usually completes to > + std::vector<...>::push_back, std::string::push_back etc. In such > + case, the symbol comparison routine will set the LCD match to point > + into the "push_back" substring within the symbol's name string. */ [missing newline?] > +class completion_match_for_lcd > +{ > +public: > + /* Set the match for LCD. See m_match's description. */ > + void set_match (const char *match) > + { m_match = match; } > + > + /* Get the resulting LCD, after a successful match. */ > + const char *finish () > + { return m_match; } > + > + /* Prepare for another completion matching sequence. */ > + void clear () > + { m_match = NULL; } > + > +private: > + /* The completion match result for LCD. This is usually either a > + pointer into to a substring within a symbol's name, or to the > + storage of the pairing completion_match object. */ > + const char *m_match; > +}; > + > /* Convenience aggregate holding info returned by the symbol name > matching routines (see symbol_name_matcher_ftype). */ > struct completion_match_result > { > /* The completion match candidate. */ > completion_match match; > + > + /* The completion match, for LCD computation purposes. */ > + completion_match_for_lcd match_for_lcd; > }; > > /* The final result of a completion that is handed over to either > diff --git a/gdb/cp-support.c b/gdb/cp-support.c > index 064a45b..397c738 100644 > --- a/gdb/cp-support.c > +++ b/gdb/cp-support.c > @@ -1660,8 +1660,11 @@ cp_fq_symbol_name_matches (const char *symbol_search_name, > name.c_str (), name.size (), > mode) == 0) > { > - if (match != NULL) > - match->set_match (symbol_search_name); > + if (comp_match_res != NULL) > + { > + comp_match_res->match.set_match (symbol_search_name); > + comp_match_res->match_for_lcd.set_match (symbol_search_name); > + } > return true; > } > > diff --git a/gdb/language.c b/gdb/language.c > index 6705a3b..511a86f 100644 > --- a/gdb/language.c > +++ b/gdb/language.c > @@ -725,8 +725,11 @@ default_symbol_name_matcher (const char *symbol_search_name, > if (strncmp_iw_with_mode (symbol_search_name, name.c_str (), name.size (), > mode) == 0) > { > - if (match != NULL) > - match->set_match (symbol_search_name); > + if (comp_match_res != NULL) > + { > + comp_match_res->match.set_match (symbol_search_name); > + comp_match_res->match_for_lcd.set_match (symbol_search_name); > + } > return true; > } > else In the above two hunks, the code calls result->match.set_match (A) and result->match_for_lcd.set_match (A), i.e., both these calls take the same argument and are called at the same time. Furthermore, on the final series of the patch, $ grep -n set_match\ \( *.c ada-lang.c:6416: comp_match_res->match.set_match (match_str.c_str ()); ada-lang.c:6417: comp_match_res->match_for_lcd.set_match (match_str.c_str ()); ada-lang.c:6426: comp_match_res->match.set_match (match_str.c_str ()); ada-lang.c:6427: comp_match_res->match_for_lcd.set_match (match_str.c_str ()); cp-support.c:1734: comp_match_res->match.set_match (symbol_search_name); cp-support.c:1735: comp_match_res->match_for_lcd.set_match (sname); cp-support.c:1773: comp_match_res->match.set_match (symbol_search_name); cp-support.c:1774: comp_match_res->match_for_lcd.set_match (symbol_search_name); language.c:725: comp_match_res->match.set_match (symbol_search_name); language.c:726: comp_match_res->match_for_lcd.set_match (symbol_search_name); It appears that these two are /always/ called together, suggesting that completion_match_result might "benefit" from a convenience method to set both of these parameters at the same time. [Unless there is a specific reason not to do this, of course.] WDYT? > diff --git a/gdb/symtab.h b/gdb/symtab.h > index d68eed8..736fea0 100644 > --- a/gdb/symtab.h > +++ b/gdb/symtab.h > @@ -292,15 +292,21 @@ private: > > SYMBOL_SEARCH_NAME should be a symbol's "search" name. > > - On success and if non-NULL, MATCH is set to point to the symbol > - name as should be presented to the user as a completion match list > - element. In most languages, this is the same as the symbol's > - search name, but in some, like Ada, the display name is dynamically > - computed within the comparison routine. */ > + On success and if non-NULL, COMP_MATCH_RES->match is set to point > + to the symbol name as should be presented to the user as a > + completion match list element. In most languages, this is the same > + as the symbol's search name, but in some, like Ada, the display > + name is dynamically computed within the comparison routine. > + > + Also, on success and if non-NULL, COMP_MATCH_RES->match_for_lcd > + points the part of SYMBOL_SEARCH_NAME that was considered to match ^ missing "to" > + LOOKUP_NAME. E.g., in C++, in linespec/wild mode, if the symbol is > + "foo::function()" and LOOKUP_NAME is "function(", MATCH_FOR_LCD > + points to "function()" inside SYMBOL_SEARCH_NAME. */ > typedef bool (symbol_name_matcher_ftype) > (const char *symbol_search_name, > const lookup_name_info &lookup_name, > - completion_match *match); > + completion_match_result *comp_match_res); > > /* Some of the structures in this file are space critical. > The space-critical structures are: > Keith