From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 9C8B8382C5F2 for ; Tue, 24 May 2022 14:15:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9C8B8382C5F2 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-41-2p1V0qGVM9aZ8PTzroSerQ-1; Tue, 24 May 2022 10:15:36 -0400 X-MC-Unique: 2p1V0qGVM9aZ8PTzroSerQ-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id AA1CC804198; Tue, 24 May 2022 14:15:35 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.106]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 69431401E3B; Tue, 24 May 2022 14:15:35 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 24OEFWpv2107143 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 24 May 2022 16:15:32 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 24OEFVMJ2107142; Tue, 24 May 2022 16:15:31 +0200 Date: Tue, 24 May 2022 16:15:31 +0200 From: Jakub Jelinek To: Julian Brown Cc: gcc-patches@gcc.gnu.org, Thomas Schwinge , Tobias Burnus , Fortran List Subject: Re: [PATCH v2 06/11] OpenMP: lvalue parsing for map clauses (C++) Message-ID: Reply-To: Jakub Jelinek References: <62e4e371468638d2f155c528a5c1e597558a56ac.1647619144.git.julian@codesourcery.com> MIME-Version: 1.0 In-Reply-To: <62e4e371468638d2f155c528a5c1e597558a56ac.1647619144.git.julian@codesourcery.com> X-Scanned-By: MIMEDefang 2.85 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, 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: fortran@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Fortran mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 May 2022 14:15:41 -0000 On Fri, Mar 18, 2022 at 09:26:47AM -0700, Julian Brown wrote: > --- a/gcc/cp/parser.cc > +++ b/gcc/cp/parser.cc > @@ -4266,6 +4266,9 @@ cp_parser_new (cp_lexer *lexer) > parser->omp_declare_simd = NULL; > parser->oacc_routine = NULL; > > + /* Allow array slice in expression. */ Better /* Disallow OpenMP array sections in expressions. */ > + parser->omp_array_section_p = false; > + > /* Not declaring an implicit function template. */ > parser->auto_is_implicit_function_template_parm_p = false; > parser->fully_implicit_function_template_p = false; I think we should figure out when we should temporarily disable parser->omp_array_section_p = false; and restore it afterwards to a saved value. E.g. cp_parser_lambda_expression seems like a good candidate, the fact that OpenMP array sections are allowed say in map clause doesn't mean they are allowed inside of lambdas and it would be especially hard when the lambda is defining a separate function and the search for OMP_ARRAY_SECTION probably wouldn't be able to discover those. Other spots to consider might be statement expressions, perhaps type definitions etc. > @@ -8021,6 +8024,7 @@ cp_parser_postfix_open_square_expression (cp_parser *parser, > releasing_vec expression_list = NULL; > location_t loc = cp_lexer_peek_token (parser->lexer)->location; > bool saved_greater_than_is_operator_p; > + bool saved_colon_corrects_to_scope_p; > > /* Consume the `[' token. */ > cp_lexer_consume_token (parser->lexer); > @@ -8028,6 +8032,9 @@ cp_parser_postfix_open_square_expression (cp_parser *parser, > saved_greater_than_is_operator_p = parser->greater_than_is_operator_p; > parser->greater_than_is_operator_p = true; > > + saved_colon_corrects_to_scope_p = parser->colon_corrects_to_scope_p; > + parser->colon_corrects_to_scope_p = false; I think the last above line should be guarded on if (parser->omp_array_section_p) There is no reason to get worse diagnostics in non-OpenMP code or even in OpenMP code where array sections aren't allowed. > + > + /* NOTE: We are reusing using the type of the whole array as the type of > + the array section here, which isn't necessarily entirely correct. > + Might need revisiting. */ "reusing using" looks weird. As for the type of OMP_ARRAY_SECTION trees, perhaps we could initially use an incomplete array (so array element would be meaningful) and when we figure out the details and the array section is contiguous change its type to array type covering it. > + return build3_loc (input_location, OMP_ARRAY_SECTION, > + TREE_TYPE (postfix_expression), > + postfix_expression, index, length); > + } > + > + parser->colon_corrects_to_scope_p = saved_colon_corrects_to_scope_p; > + > /* Look for the closing `]'. */ > cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE); > > @@ -36536,7 +36570,7 @@ struct omp_dim > static tree > cp_parser_omp_var_list_no_open (cp_parser *parser, enum omp_clause_code kind, > tree list, bool *colon, > - bool allow_deref = false) > + bool map_lvalue = false) > { > auto_vec dims; > bool array_section_p; > @@ -36547,12 +36581,95 @@ cp_parser_omp_var_list_no_open (cp_parser *parser, enum omp_clause_code kind, > parser->colon_corrects_to_scope_p = false; > *colon = false; > } > + begin_scope (sk_omp, NULL); Why? Base-language-wise, clauses don't introduce a new scope for name-lookup. And if it is really needed, I'd strongly prefer to either do it solely for the clauses that might need it, or do begin_scope before first such clause and finish at the end if it has been introduced. > while (1) > { > tree name, decl; > > if (kind == OMP_CLAUSE_DEPEND || kind == OMP_CLAUSE_AFFINITY) > cp_parser_parse_tentatively (parser); > + else if (map_lvalue && kind == OMP_CLAUSE_MAP) > + { This shouldn't be done just for OMP_CLAUSE_MAP, but for all the other clauses that accept array sections, including OMP_CLAUSE_DEPEND, OMP_CLAUSE_AFFINITY, OMP_CLAUSE_MAP, OMP_CLAUSE_TO, OMP_CLAUSE_FROM, OMP_CLAUSE_INCLUSIVE, OMP_CLAUSE_EXCLUSIVE, OMP_CLAUSE_USE_DEVICE_ADDR, OMP_CLAUSE_HAS_DEVICE_ADDR, OMP_CLAUSE_*REDUCTION. And preferrably, they should be kept in the IL until *finish_omp_clauses, which should handle those instead of TREE_LIST that represented them before. Additionally, something should diagnose incorrect uses of OMP_ARRAY_SECTION, which is everywhere in the expressions but as the outermost node(s), i.e. for clauses that do allow array sections scan OMP_CLAUSE_DECL after handling handleable array sections and complain about embedded OMP_ARRAY_SECTION, including OMP_ARRAY_SECTION say in the lower-bound, length and/or stride expressions of the valid OMP_ARRAY_SECTION. For C++ that also means handling OMP_ARRAY_SECTION code in pt.c. Jakub