From: Jason Merrill <jason@redhat.com>
To: Marek Polacek <polacek@redhat.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: C++ PATCH to implement deferred parsing of noexcept-specifiers (c++/86476, c++/52869)
Date: Tue, 28 May 2019 15:48:00 -0000 [thread overview]
Message-ID: <2a193c49-75c7-4407-1f1f-3748a69ab560@redhat.com> (raw)
In-Reply-To: <20190510192133.GC20687@redhat.com>
On 5/10/19 3:21 PM, Marek Polacek wrote:
> Coming back to this. I didn't think this was suitable for GCC 9.
>
> On Mon, Jan 07, 2019 at 10:44:37AM -0500, Jason Merrill wrote:
>> On 12/19/18 3:27 PM, Marek Polacek wrote:
>>> Prompted by Jon's observation in 52869, I noticed that we don't treat
>>> a noexcept-specifier as a complete-class context of a class ([class.mem]/6).
>>> As with member function bodies, default arguments, and NSDMIs, names used in
>>> a noexcept-specifier of a member-function can be declared later in the class
>>> body, so we need to wait and parse them at the end of the class.
>>> For that, I've made use of DEFAULT_ARG (now best to be renamed to UNPARSED_ARG).
>>
>> Or DEFERRED_PARSE, yes.
>
> I didn't change the name but I'm happy to do it as a follow up.
>
>>> + /* We can't compare unparsed noexcept-specifiers. Save the old decl
>>> + and check this again after we've parsed the noexcept-specifiers
>>> + for real. */
>>> + if (UNPARSED_NOEXCEPT_SPEC_P (new_exceptions))
>>> + {
>>> + vec_safe_push (DEFARG_INSTANTIATIONS (TREE_PURPOSE (new_exceptions)),
>>> + copy_decl (old_decl));
>>> + return;
>>> + }
>>
>> Why copy_decl?
>
> This is so that we don't lose the diagnostic in noexcept46.C. If I don't use
> copy_decl then the tree is shared and subsequent changes to it make us not
> detect discrepancies like noexcept(false) vs. noexcept(true) on the same decl.
OK, fair enough.
> @@ -20515,7 +20524,13 @@ cp_parser_init_declarator (cp_parser* parser,
> /*asmspec=*/NULL_TREE,
> attr_chainon (attributes, prefix_attributes));
> if (decl && TREE_CODE (decl) == FUNCTION_DECL)
> - cp_parser_save_default_args (parser, decl);
> + {
> + cp_parser_save_default_args (parser, decl);
> + /* Remember if there is a noexcept-specifier to post process. */
> + tree spec = TYPE_RAISES_EXCEPTIONS (TREE_TYPE (decl));
> + if (UNPARSED_NOEXCEPT_SPEC_P (spec))
> + vec_safe_push (unparsed_noexcepts, decl);
Can we handle this in cp_parser_save_default_args rather than all its
callers?
> +/* Make sure that any member-function parameters are in scope.
> + For instance, a function's noexcept-specifier can use the function's
> + parameters:
> +
> + struct S {
> + void fn (int p) noexcept(noexcept(p));
> + };
> +
> + so we need to make sure name lookup can find them. This is used
> + when we delay parsing of the noexcept-specifier. */
> +
> +static void
> +inject_parm_decls (tree decl)
> +{
> + begin_scope (sk_function_parms, decl);
> + tree args = DECL_ARGUMENTS (decl);
> + args = nreverse (args);
> +
> + tree next;
> + for (tree parm = args; parm; parm = next)
> + {
> + next = DECL_CHAIN (parm);
> + if (TREE_CODE (parm) == PARM_DECL)
> + pushdecl (parm);
> + }
> + /* Get the decls in their original chain order and record in the
> + function. This is all and only the PARM_DECLs that were
> + pushed into scope by the loop above. */
> + DECL_ARGUMENTS (decl) = get_local_decls ();
> +}
Can we share this code with store_parm_decls instead of having two copies?
> @@ -25227,6 +25431,18 @@ cp_parser_noexcept_specification_opt (cp_parser* parser,
> + /* [class.mem]/6 says that a noexcept-specifer (within the
> + member-specification of the class) is a complete-class context of
> + a class. So, if the noexcept-specifier has the optional expression,
> + just save the tokens, and reparse this after we're done with the
> + class. */
> + if (cp_lexer_peek_nth_token (parser->lexer, 2)->type == CPP_OPEN_PAREN
> + && at_class_scope_p ()
> + && TYPE_BEING_DEFINED (current_class_type)
> + && !LAMBDA_TYPE_P (current_class_type))
> + return cp_parser_save_noexcept (parser);
We might optimize the noexcept(<literal>) case, which should be pretty
common.
> +maybe_check_throw_specifier (tree overrider, tree basefn)
maybe_check_overriding_exception_spec
> diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c
> index df002a1664c..8cbc48fb44f 100644
> --- gcc/cp/typeck2.c
> +++ gcc/cp/typeck2.c
> @@ -2393,6 +2393,12 @@ merge_exception_specifiers (tree list, tree add)
> if (list == error_mark_node || add == error_mark_node)
> return error_mark_node;
>
> + /* We don't want to lose the unparsed operand lest we miss diagnostics. */
> + if (UNPARSED_NOEXCEPT_SPEC_P (list))
> + return list;
> + else if (UNPARSED_NOEXCEPT_SPEC_P (add))
> + return add;
Here you're throwing away the other side, which seems wrong.
Jason
next prev parent reply other threads:[~2019-05-28 15:47 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-19 20:27 Marek Polacek
2019-01-04 14:45 ` Marek Polacek
2019-01-07 15:44 ` Jason Merrill
2019-05-10 19:21 ` Marek Polacek
2019-05-17 14:35 ` Marek Polacek
2019-05-24 16:17 ` Marek Polacek
2019-05-28 15:48 ` Jason Merrill [this message]
2019-06-04 1:02 ` Marek Polacek
2019-06-10 12:28 ` Marek Polacek
2019-06-12 3:46 ` Jason Merrill
2019-06-14 20:54 ` Marek Polacek
2019-06-21 20:47 ` Jason Merrill
2019-06-21 21:30 ` Marek Polacek
2019-06-22 0:28 ` Jason Merrill
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2a193c49-75c7-4407-1f1f-3748a69ab560@redhat.com \
--to=jason@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=polacek@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).