From patchwork Fri Nov 24 11:32:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathaniel Shead X-Patchwork-Id: 1868126 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=jFWHBkr3; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4ScCWx0dSVz1yR8 for ; Fri, 24 Nov 2023 22:32:45 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 54ECC3857B85 for ; Fri, 24 Nov 2023 11:32:42 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pf1-x42b.google.com (mail-pf1-x42b.google.com [IPv6:2607:f8b0:4864:20::42b]) by sourceware.org (Postfix) with ESMTPS id CD54B385C332 for ; Fri, 24 Nov 2023 11:32:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CD54B385C332 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org CD54B385C332 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::42b ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700825550; cv=none; b=vT/Cn3bkJiSwZl+n3daCzVDRmYtAZPGiaPelOWTIfB1/rz0jEGgpxbHk/Bj5WvMZ26RKp6zT3fViS4gJIJJOllpKMPP0e7cSynUghEmbgpU2UeNw6/pFNQQi6dsA+Gkad2Z/NZ8tjBquriuLfynNTh8MjcMHAUbMRgcMYKvKFbw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700825550; c=relaxed/simple; bh=qsKGjUz68uVd1JDuJhB7gUVuXHi1hnMJk2HvdWzhVPU=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=onaD3G9cfSsa+zeN/olRufRv99PzFewxSFW8Mh2BPIgt72Ps6K3x7CiJa9JCHCx6HUF1DB9qllUl6nBOty1uVKt22pHOqrmqSlnOHn98Wf9j00oaU3h1V7kiTKioRUjVHSrZ9i38kPHZcm5wwcDbt+7fXVOyEhXviaePJuVxfAE= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x42b.google.com with SMTP id d2e1a72fcca58-6b709048d8eso1625807b3a.2 for ; Fri, 24 Nov 2023 03:32:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1700825539; x=1701430339; darn=gcc.gnu.org; h=in-reply-to:content-disposition:mime-version:references:subject:cc :to:from:date:message-id:from:to:cc:subject:date:message-id:reply-to; bh=Txk4H+kKWVjyKYEnNyNa5RRrkBzcbzotDYlS743j9ho=; b=jFWHBkr3UtBsHwF3xYdtU+m/Mvplwlx9Ma/n0vlMBNh7r5X+9QUOqbKzQJnFCKXGov 0epYYWZ09qDeWoyYtDn9A3xYfiaZaJ26XSpznxdKpcJHx9kr+2HHJ9AKvFDyz4g3cp6R 5WJS0STlUJaFoG7iOoScAhpUNHk31US9CQrEd3s49+OBoIV6I6eNjGcMMn2jOQ0OAnIC TClmyyVI1HRAL+IV2PvIoP6oSuY+HeH6GHrbQ2uRRdKyHeVGz7CVChXM0DHBgJEGMskm wN2m6LEyG0Z8VoYEFP2FelUHpoUnbp/uDteJu4eKKDIeE1Ks7P66osabH8DPi4lBkCh4 kcBQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700825539; x=1701430339; h=in-reply-to:content-disposition:mime-version:references:subject:cc :to:from:date:message-id:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=Txk4H+kKWVjyKYEnNyNa5RRrkBzcbzotDYlS743j9ho=; b=M499LvpLIjsSltm8xGd4aRzyphfNkmEG95QFxJWw7EK9moOy3A1bRBlj3Is4uLeK8/ tbVL8hrOAr0ezAiImjxmS9pe3L5/nqWl5WGjtG5EixfDs7AX1YkQy5Uz6SM3HIx0iohH 3tC8kItSrQVGb9MdqBcZMz/6m71aiUSzp9PiCtn7SJ/1axXr6kttcdZm2lG5PnJp70qt ZGFtq4FO2FbDXA1cKJfGCG5rOqD9LL61q+Vo+kNIxG38DG1JKu/8MgeGp3Zai7CwoFzi nxCUii1kKRK6kDk7pNlDpPHn4zx1XSuLS0i5AhUOAhlnkmik7x8Onj1Pmo362tZbU7WT 2OEA== X-Gm-Message-State: AOJu0YwitCDXJaagaUbe0Ghbo8cEJwGEyoXUdnwlDWOoc0JACB0dpd9Z Q8iHJpK1NE4AlvVkzu0cmNdJmAcxlGM= X-Google-Smtp-Source: AGHT+IHitqEXeBt54bA1jDF2asNqHZT2t8hGazwJmvqk75uiQ6ZF0Z4REwWfAHmQttBoRQ7/eG+F4w== X-Received: by 2002:a05:6a20:9145:b0:18b:5a8a:4333 with SMTP id x5-20020a056a20914500b0018b5a8a4333mr2685086pzc.19.1700825538448; Fri, 24 Nov 2023 03:32:18 -0800 (PST) Received: from Thaum. ([124.168.116.122]) by smtp.gmail.com with ESMTPSA id u26-20020a62ed1a000000b006cbcd08ed56sm2683664pfh.56.2023.11.24.03.32.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Nov 2023 03:32:18 -0800 (PST) Message-ID: <656089c2.620a0220.3437d.6417@mx.google.com> X-Google-Original-Message-ID: Date: Fri, 24 Nov 2023 22:32:13 +1100 From: Nathaniel Shead To: Nathan Sidwell Cc: gcc-patches@gcc.gnu.org, Jason Merrill Subject: [PATCH v2] c++: Follow module grammar more closely [PR110808] References: <655312af.630a0220.8e418.ba89@mx.google.com> <4b2df9c9-6658-4240-9f4f-4f7197c64490@acm.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4b2df9c9-6658-4240-9f4f-4f7197c64490@acm.org> X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org On Thu, Nov 23, 2023 at 12:11:58PM -0500, Nathan Sidwell wrote: > On 11/14/23 01:24, Nathaniel Shead wrote: > > I'll also note that the comments above the parsing functions here no > > longer exactly match with the grammar in the standard, should they be > > updated as well? > > please. > As I was attempting to rewrite the comments I ended up splitting up the work that was being done by cp_parser_module_name a lot to better match the grammar, and also caught a few other segfaults that were occurring along the way. Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- This patch cleans up the parsing of module-declarations and import-declarations to more closely follow the grammar defined by the standard. For instance, currently we allow declarations like 'import A:B', even from an unrelated source file (not part of module A), which causes errors in merging declarations. However, the syntax in [module.import] doesn't even allow this form of import, so this patch prevents this from parsing at all and avoids the error that way. Additionally, we sometimes allow statements like 'import :X' or 'module :X' even when not in a named module, and this causes segfaults, so we disallow this too. PR c++/110808 gcc/cp/ChangeLog: * parser.cc (cp_parser_module_name): Rewrite to handle module-names and module-partitions independently. (cp_parser_module_partition): New function. (cp_parser_module_declaration): Parse module partitions explicitly. Don't change state if parsing module decl failed. (cp_parser_import_declaration): Handle different kinds of import-declarations locally. gcc/testsuite/ChangeLog: * g++.dg/modules/part-hdr-1_c.C: Fix syntax. * g++.dg/modules/part-mac-1_c.C: Likewise. * g++.dg/modules/mod-invalid-1.C: New test. * g++.dg/modules/part-8_a.C: New test. * g++.dg/modules/part-8_b.C: New test. * g++.dg/modules/part-8_c.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/parser.cc | 100 ++++++++++++------- gcc/testsuite/g++.dg/modules/mod-invalid-1.C | 7 ++ gcc/testsuite/g++.dg/modules/part-8_a.C | 6 ++ gcc/testsuite/g++.dg/modules/part-8_b.C | 6 ++ gcc/testsuite/g++.dg/modules/part-8_c.C | 8 ++ gcc/testsuite/g++.dg/modules/part-hdr-1_c.C | 2 +- gcc/testsuite/g++.dg/modules/part-mac-1_c.C | 2 +- 7 files changed, 95 insertions(+), 36 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/mod-invalid-1.C create mode 100644 gcc/testsuite/g++.dg/modules/part-8_a.C create mode 100644 gcc/testsuite/g++.dg/modules/part-8_b.C create mode 100644 gcc/testsuite/g++.dg/modules/part-8_c.C diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index f6d088bc73f..20bd8d45a08 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -14853,58 +14853,64 @@ cp_parser_already_scoped_statement (cp_parser* parser, bool *if_p, /* Modules */ -/* Parse a module-name, - identifier - module-name . identifier - header-name +/* Parse a module-name or module-partition. - Returns a pointer to module object, NULL. */ + module-name: + module-name-qualifier [opt] identifier -static module_state * -cp_parser_module_name (cp_parser *parser) -{ - cp_token *token = cp_lexer_peek_token (parser->lexer); - if (token->type == CPP_HEADER_NAME) - { - cp_lexer_consume_token (parser->lexer); + module-partition: + : module-name-qualifier [opt] identifier - return get_module (token->u.value); - } + module-name-qualifier: + identifier . + module-name-qualifier identifier . - module_state *parent = NULL; - bool partitioned = false; - if (token->type == CPP_COLON && named_module_p ()) - { - partitioned = true; - cp_lexer_consume_token (parser->lexer); - } + Returns a pointer to the module object, or NULL on failure. + For PARTITION_P, PARENT is the module this is a partition of. */ + +static module_state * +cp_parser_module_name (cp_parser *parser, bool partition_p = false, + module_state *parent = NULL) +{ + if (partition_p + && cp_lexer_consume_token (parser->lexer)->type != CPP_COLON) + return NULL; for (;;) { if (cp_lexer_peek_token (parser->lexer)->type != CPP_NAME) { - cp_parser_error (parser, "expected module-name"); - break; + if (partition_p) + cp_parser_error (parser, "expected module-partition"); + else + cp_parser_error (parser, "expected module-name"); + return NULL; } tree name = cp_lexer_consume_token (parser->lexer)->u.value; - parent = get_module (name, parent, partitioned); - token = cp_lexer_peek_token (parser->lexer); - if (!partitioned && token->type == CPP_COLON) - partitioned = true; - else if (token->type != CPP_DOT) + parent = get_module (name, parent, partition_p); + if (cp_lexer_peek_token (parser->lexer)->type != CPP_DOT) break; cp_lexer_consume_token (parser->lexer); - } + } return parent; } +/* Parse a module-partition. Defers to cp_parser_module_name. */ + +static module_state * +cp_parser_module_partition (cp_parser *parser, module_state *parent = NULL) +{ + return cp_parser_module_name (parser, /*partition_p=*/true, parent); +} + /* Named module-declaration __module ; PRAGMA_EOL - __module private ; PRAGMA_EOL (unimplemented) - [__export] __module module-name attr-spec-seq-opt ; PRAGMA_EOL + __module : private ; PRAGMA_EOL (unimplemented) + [__export] __module module-name module-partition [opt] + attr-spec-seq-opt ; PRAGMA_EOL */ static module_parse @@ -14962,9 +14968,12 @@ cp_parser_module_declaration (cp_parser *parser, module_parse mp_state, else { module_state *mod = cp_parser_module_name (parser); + if (mod && cp_lexer_peek_token (parser->lexer)->type == CPP_COLON) + mod = cp_parser_module_partition (parser, mod); tree attrs = cp_parser_attributes_opt (parser); - mp_state = MP_PURVIEW_IMPORTS; + if (mod) + mp_state = MP_PURVIEW_IMPORTS; if (!mod || !cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON)) goto skip_eol; @@ -14976,7 +14985,10 @@ cp_parser_module_declaration (cp_parser *parser, module_parse mp_state, } /* Import-declaration - [__export] __import module-name attr-spec-seq-opt ; PRAGMA_EOL */ + __import module-name attr-spec-seq-opt ; PRAGMA_EOL + __import module-partition attr-spec-seq-opt ; PRAGMA_EOL + __import header-name attr-spec-seq-opt ; PRAGMA_EOL +*/ static void cp_parser_import_declaration (cp_parser *parser, module_parse mp_state, @@ -15004,7 +15016,27 @@ cp_parser_import_declaration (cp_parser *parser, module_parse mp_state, } else { - module_state *mod = cp_parser_module_name (parser); + module_state *mod = NULL; + cp_token *next = cp_lexer_peek_token (parser->lexer); + if (next->type == CPP_HEADER_NAME) + { + cp_lexer_consume_token (parser->lexer); + mod = get_module (next->u.value); + } + else if (next->type == CPP_COLON) + { + /* An import specifying a module-partition shall only appear after the + module-declaration in a module unit: [module.import]/4. */ + if (named_module_p () + && (mp_state == MP_PURVIEW_IMPORTS + || mp_state == MP_PRIVATE_IMPORTS)) + mod = cp_parser_module_partition (parser); + else + error_at (next->location, "import specifying a module-partition" + " must appear after a named module-declaration"); + } + else + mod = cp_parser_module_name (parser); tree attrs = cp_parser_attributes_opt (parser); if (!mod || !cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON)) diff --git a/gcc/testsuite/g++.dg/modules/mod-invalid-1.C b/gcc/testsuite/g++.dg/modules/mod-invalid-1.C new file mode 100644 index 00000000000..fadaba0b560 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/mod-invalid-1.C @@ -0,0 +1,7 @@ +// { dg-additional-options "-fmodules-ts" } + +module; + +module :foo; // { dg-error "expected module-name" } + +import :foo; // { dg-error "import specifying a module-partition must appear after a named module-declaration" } diff --git a/gcc/testsuite/g++.dg/modules/part-8_a.C b/gcc/testsuite/g++.dg/modules/part-8_a.C new file mode 100644 index 00000000000..09f956ff36f --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/part-8_a.C @@ -0,0 +1,6 @@ +// PR c++/110808 +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi group:tres } + +export module group:tres; +int mul() { return 0; } diff --git a/gcc/testsuite/g++.dg/modules/part-8_b.C b/gcc/testsuite/g++.dg/modules/part-8_b.C new file mode 100644 index 00000000000..1ade029495c --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/part-8_b.C @@ -0,0 +1,6 @@ +// PR c++/110808 +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi group } + +export module group; +export import :tres; diff --git a/gcc/testsuite/g++.dg/modules/part-8_c.C b/gcc/testsuite/g++.dg/modules/part-8_c.C new file mode 100644 index 00000000000..2351f28f909 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/part-8_c.C @@ -0,0 +1,8 @@ +// PR c++/110808 +// { dg-additional-options "-fmodules-ts" } + +import group:tres; // { dg-error "expected .;." } + +int main() { + return mul(); // { dg-error "not declared" } +} diff --git a/gcc/testsuite/g++.dg/modules/part-hdr-1_c.C b/gcc/testsuite/g++.dg/modules/part-hdr-1_c.C index 78a53d2fda3..db57adcef44 100644 --- a/gcc/testsuite/g++.dg/modules/part-hdr-1_c.C +++ b/gcc/testsuite/g++.dg/modules/part-hdr-1_c.C @@ -2,4 +2,4 @@ // { dg-module-cmi {mod} } export module mod; -import mod:impl; +import :impl; diff --git a/gcc/testsuite/g++.dg/modules/part-mac-1_c.C b/gcc/testsuite/g++.dg/modules/part-mac-1_c.C index 78a53d2fda3..db57adcef44 100644 --- a/gcc/testsuite/g++.dg/modules/part-mac-1_c.C +++ b/gcc/testsuite/g++.dg/modules/part-mac-1_c.C @@ -2,4 +2,4 @@ // { dg-module-cmi {mod} } export module mod; -import mod:impl; +import :impl;