public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "cvs-commit at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c++/98672] constexpr function - for loop with return statement doesn't get recognized as constexpr subexpression
Date: Thu, 21 Jan 2021 16:23:16 +0000	[thread overview]
Message-ID: <bug-98672-4-ThPdzCH7GA@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-98672-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98672

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:0fb7aa205afebe178c06683037ccd4c41104337a

commit r11-6840-g0fb7aa205afebe178c06683037ccd4c41104337a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jan 21 17:20:24 2021 +0100

    c++: Fix up potential_constant_expression_1 FOR/WHILE_STMT handling
[PR98672]

    The following testcase is rejected even when it is valid.
    The problem is that potential_constant_expression_1 doesn't have the
    accurate *jump_target tracking cxx_eval_* has, and when the loop has
    a condition that isn't guaranteed to be always true, the body isn't walked
    at all.  That is mostly a correct conservative behavior, except that it
    doesn't detect if there are any return statements in the body, which means
    the loop might return instead of falling through to the next statement.
    We already have code for return stmt discovery in code snippets we don't
    try to evaluate for switches, so this patch reuses that for FOR_STMT
    and WHILE_STMT bodies.

    Note, I haven't touched FOR_EXPR, with statement expressions it could
    have return stmts in it too, or it could have break or continue statements
    that wouldn't bind to the current loop but to something outer.  That
    case is clearly mishandled by potential_constant_expression_1 even
    when the condition is missing or is always true, and it wouldn't surprise
me
    if cxx_eval_* didn't handle it right either, so I'm deferring that to
    separate PR for later.  We'd need proper test coverage for all of that.

    > Hmm, IF_STMT probably also needs to check the else clause, if the
condition
    > isn't a known constant.

    You're right, I thought it was ok because it recurses with tf_none, but
    if the then branch is potentially constant and only else returns, continues
    or breaks, then as the enhanced testcase shows we were mishandling it too.

    2021-01-21  Jakub Jelinek  <jakub@redhat.com>

            PR c++/98672
            * constexpr.c (check_for_return_continue_data): Add break_stmt
member.
            (check_for_return_continue): Also look for BREAK_STMT.  Handle
            SWITCH_STMT by ignoring break_stmt from its body.
            (potential_constant_expression_1) <case FOR_STMT>,
            <case WHILE_STMT>: If the condition isn't constant true, check if
            the loop body can contain a return stmt.
            <case SWITCH_STMT>: Adjust check_for_return_continue_data
initializer.
            <case IF_STMT>: If recursion with tf_none is successful,
            merge *jump_target from the branches - returns with highest
priority,
            breaks or continues lower.  If then branch is potentially constant
and
            doesn't return, check the else branch if it could return, break or
            continue.

            * g++.dg/cpp1y/constexpr-98672.C: New test.

  parent reply	other threads:[~2021-01-21 16:23 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-14  9:17 [Bug c++/98672] New: " ryan_greenblatt at brown dot edu
2021-01-14 11:48 ` [Bug c++/98672] " jakub at gcc dot gnu.org
2021-01-21 16:23 ` cvs-commit at gcc dot gnu.org [this message]
2021-01-21 16:28 ` jakub at gcc dot gnu.org
2021-01-29 19:19 ` cvs-commit at gcc dot gnu.org
2021-04-20 23:31 ` cvs-commit at gcc dot gnu.org
2021-04-22 14:12 ` jakub at gcc dot gnu.org
2021-08-28 21:45 ` pinskia at gcc dot gnu.org
2021-08-28 21:46 ` pinskia at gcc dot gnu.org

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=bug-98672-4-ThPdzCH7GA@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /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).