From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 59394 invoked by alias); 13 Jul 2017 20:41:19 -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 59383 invoked by uid 89); 13 Jul 2017 20:41:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= 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; Thu, 13 Jul 2017 20:41:17 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 841474D4A2 for ; Thu, 13 Jul 2017 20:41:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 841474D4A2 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=keiths@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 841474D4A2 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 DBCA46060C; Thu, 13 Jul 2017 20:41:14 +0000 (UTC) Subject: Re: [PATCH 04/40] Fix TAB-completion + .gdb_index slowness (generalize filename_seen_cache) To: Pedro Alves , gdb-patches@sourceware.org References: <1496406158-12663-1-git-send-email-palves@redhat.com> <1496406158-12663-5-git-send-email-palves@redhat.com> From: Keith Seitz Message-ID: <5967DAEA.1060304@redhat.com> Date: Thu, 13 Jul 2017 20:41:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <1496406158-12663-5-git-send-email-palves@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-07/txt/msg00169.txt.bz2 I just have a little comment... On 06/02/2017 05:22 AM, Pedro Alves wrote: > diff --git a/gdb/filename-seen-cache.h b/gdb/filename-seen-cache.h > new file mode 100644 > index 0000000..c041d3e > --- /dev/null > +++ b/gdb/filename-seen-cache.h > @@ -0,0 +1,64 @@ > +/* Filename-seen cache for the GNU debugger, GDB. > + > + Copyright (C) 1986-2017 Free Software Foundation, Inc. > + > + This file is part of GDB. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +#include "defs.h" > +#include "common/function-view.h" > + > +/* Cache to watch for file names already seen. */ > + > +class filename_seen_cache > +{ > +public: > + filename_seen_cache (); > + ~filename_seen_cache (); > + > + /* Disable copy. */ > + filename_seen_cache (const filename_seen_cache &) = delete; > + void operator= (const filename_seen_cache &) = delete; > + > + /* Empty the cache, but do not delete it. */ > + void clear (); > + > + /* If FILE is not already in the table of files in CACHE, return > + false; otherwise return true. Optionally add FILE to the table > + if ADD is true. > + > + NOTE: We don't manage space for FILE, we assume FILE lives as > + long as the caller needs. */ > + bool filename_seen (const char *file, bool add); I'm not really a fan of this style of interface. [I realize we've done this for many years.] When reading through code that uses this, e.g., bool seen = cache.filename_seen (file, true); that "true" doesn't really tell me what that means, and in the context of the method itself, it isn't obvious (to me, at least) what "true" might mean when asking if we've seen a filename. A reader could think that it somehow altered how the search was performed, e.g., case-insensitive or whatnot. Now if this read bool seen = cache.filename_seen (file, ADD); That reveals much more about what is going to happen. But that's nitpicky style comment (aka there's no reason to change anything). I didn't otherwise see any issues with the patch. Keith