From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 116182 invoked by alias); 12 Aug 2018 00:17:02 -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 116150 invoked by uid 89); 12 Aug 2018 00:17:00 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-15.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,KAM_STOCKGEN,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=REF, UNION, sk:unique_, INT X-HELO: gateway36.websitewelcome.com Received: from gateway36.websitewelcome.com (HELO gateway36.websitewelcome.com) (192.185.193.119) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 12 Aug 2018 00:16:59 +0000 Received: from cm12.websitewelcome.com (cm12.websitewelcome.com [100.42.49.8]) by gateway36.websitewelcome.com (Postfix) with ESMTP id C9B3A400D7761 for ; Sat, 11 Aug 2018 18:20:02 -0500 (CDT) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id oe41f18RBSjJAoe41fjSvJ; Sat, 11 Aug 2018 19:16:57 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date: References:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=MAKcqdNuS+a0qh4kz4YuoYTG9QgroE5Hbu3+TXXl5Ao=; b=yXS5mVna0Fj7nkzVWkncFUBOv1 PsjekeAS2enjJ8Lx2dQ3MQg/jx6N4n/vz+jpqyzwpJltd4GK2CmStAn3ROJU61U+v9xmVDmgrh5n7 NxxvQvVcrirYykDpx+1mN4w9+; Received: from 75-166-85-72.hlrn.qwest.net ([75.166.85.72]:59388 helo=bapiya) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.91) (envelope-from ) id 1foe41-0015fW-9G; Sat, 11 Aug 2018 19:16:57 -0500 From: Tom Tromey To: Keith Seitz Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 9/9] C++ compile support References: <20180810232534.481-1-keiths@redhat.com> <20180810232534.481-10-keiths@redhat.com> Date: Sun, 12 Aug 2018 00:17:00 -0000 In-Reply-To: <20180810232534.481-10-keiths@redhat.com> (Keith Seitz's message of "Fri, 10 Aug 2018 16:25:34 -0700") Message-ID: <87tvo0sakn.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2018-08/txt/msg00325.txt.bz2 >>>>> "Keith" == Keith Seitz writes: Keith> This patch adds *basic* support for C++ to the compile feature. It does Keith> most simple type conversions, including everything that C compile does and Keith> your basic "with-classes" type of C++. Thanks for doing this. This is very good. Keith> +extern compile_instance *cplus_get_compile_context (void); I think we stopped using (void) now. Keith> +typedef compile_program + cplus_push_user_expression, cplus_pop_user_expression, Keith> + cplus_add_code_header, c_add_code_footer, Keith> + cplus_add_input> cplus_compile_program; Keith> + Haha, this seemed like overkill to me, but I see it's already this way, and it doesn't really matter I think. Keith> diff --git a/gdb/compile/compile-cplus-support.c b/gdb/compile/compile-cplus-support.c Keith> new file mode 100644 Keith> index 0000000000..42fc7692c4 [...] Keith> +void Keith> +gcc_cplus_enter_scope (void *datum, struct gcc_cp_context *gcc_context) Keith> +{ Keith> +} Keith> + Keith> +void Keith> +gcc_cplus_leave_scope (void *datum, struct gcc_cp_context *gcc_context) Keith> +{ Keith> +} Is this something that gets expanded in later patches? If not, I'd suggest just making these private static methods on compile_cplus_instance, to avoid needing a new file. On the other hand, if this is expanded in the future, then it is totally fine like this. Keith> +/* Convert a given symbol, SYM, to the compiler's representation. Keith> + INSTANCE is the compiler instance. IS_GLOBAL is true if the Keith> + symbol came from the global scope. IS_LOCAL is true if the symbol Keith> + came from a local scope. (Note that the two are not strictly Keith> + inverses because the symbol might have come from the static Keith> + scope.) */ Keith> + Keith> +static void Keith> +convert_one_symbol (compile_cplus_instance *instance, Keith> + struct block_symbol sym, bool is_global, bool is_local) I think back in the day, I'd hoped that there would be more code sharing between the C and C++ compile paths. But maybe this wasn't possible? I didn't do a deep comparison or anything. And it's fine if they have to be separate. Keith> +void _initialize_compile_cplus_symbols (void); I think this declaration isn't needed any more. Keith> +/* See description in compile-cplus.h. */ Keith> + Keith> +gdb::unique_xmalloc_ptr Keith> +decl_name (const char *natural) This should return a std::string. See below. Keith> + std::string symbol = (comp.bsymbol.symbol != nullptr Keith> + ? SYMBOL_NATURAL_NAME (comp.bsymbol.symbol) Keith> + : ""); This could just be a const char * to avoid an allocation. Keith> +/* Utility function to convert CODE into a string. */ Keith> + Keith> +static const char * Keith> +type_code_to_string (enum type_code code) Keith> +{ Keith> + const char * const s[] = Keith> + {"BISTRING (deprecated)", "UNDEF (not used)", Keith> + "PTR", "ARRAY", "STRUCT", "UNION", "ENUM", Keith> + "FLAGS", "FUNC", "INT", "FLT", "VOID", Keith> + "SET", "RANGE", "STRING", "ERROR", "METHOD", Keith> + "METHODPTR", "MEMBERPTR", "REF", "RVALUE_REF", "CHAR", "BOOL", Keith> + "COMPLEX", "TYPEDEF", "NAMESPACE", "DECFLOAT", "MODULE", Keith> + "INTERNAL_FUNCTION", "XMETHOD"}; Keith> + Keith> + return s[code + 1]; This could get out of sync with gdbtypes.h. So I'd suggest either (1) taking the simple route and printing this as an integer; or (2) moving the enum to a .defs file and auto-generating the strings from it, to avoid any possibility of problems. In case 2, type_code_to_string could move to gdbtypes.c. Keith> + scope_component comp Keith> + = { Keith> + decl_name (TYPE_NAME (type)).release (), Keith> + lookup_symbol (TYPE_NAME (type), block (), VAR_DOMAIN, nullptr) scope_component::name is a std::string, so I think this decl_name call will leak memory; which is why decl_name should return a std::string itself. Keith> +/* Return the declaration name of the symbol named NATURAL. Keith> + This returns a name with no function arguments or template parameters, Keith> + suitable for passing to the compiler plug-in. */ Keith> + Keith> +gdb::unique_xmalloc_ptr decl_name (const char *natural); This seems like a fairly generic name for something that is maybe specific to the C++ compile code. Tom