public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jason Merrill <jason@redhat.com>
To: Marek Polacek <polacek@redhat.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: C++ PATCH for c++/78244 - narrowing conversion in template not detected, part 2
Date: Mon, 21 Jan 2019 20:15:00 -0000	[thread overview]
Message-ID: <bd67f9ca-138f-f42e-1ffd-5c4cf80893f3@redhat.com> (raw)
In-Reply-To: <20190118141207.GN19569@redhat.com>

On 1/18/19 9:12 AM, Marek Polacek wrote:
> On Thu, Jan 17, 2019 at 04:17:29PM -0500, Jason Merrill wrote:
>> On 1/17/19 2:09 PM, Marek Polacek wrote:
>>> This patch ought to fix the rest of 78244, a missing narrowing warning in
>>> decltype.
>>>
>>> As I explained in Bugzilla, there can be three scenarios:
>>>
>>> 1) decltype is in a template and it has no dependent expressions, which
>>> is the problematical case.  finish_compound_literal just returns the
>>> compound literal without checking narrowing if processing_template_decl.
>>
>> This is the sort of thing that we've been gradually fixing: if the compound
>> literal isn't dependent at all, we want to do the normal processing.  And
>> then usually return a result based on the original trees rather than the
>> result of processing.  For instance, finish_call_expr.  Something like that
>> ought to work here, too, and be more generally applicable; this shouldn't be
>> limited to casting to a scalar type, casting to a known class type can also
>> involve narrowing.
> 
> Great, that works just fine.  I also had to check if the type is
> type-dependent, otherwise complete_type could fail.
> 
>> The check in the other patch that changes instantiation_dependent_r should
>> be more similar to the one in finish_compound_literal.  Or perhaps you could
>> set a flag here in finish_compound_literal to indicate that it's
>> instantiation-dependent, and just check that in instantiation_dependent_r.
> 
> Done, but I feel bad about adding another flag.  But I guess it's cheaper
> this way.  Thanks!
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 2019-01-18  Marek Polacek  <polacek@redhat.com>
> 
> 	PR c++/88815 - narrowing conversion lost in decltype.
> 	PR c++/78244 - narrowing conversion in template not detected.
> 	* cp-tree.h (CONSTRUCTOR_IS_DEPENDENT): New.
> 	* pt.c (instantiation_dependent_r): Consider a CONSTRUCTOR with
> 	CONSTRUCTOR_IS_DEPENDENT instantiation-dependent.
> 	* semantics.c (finish_compound_literal): When the compound literal
> 	isn't instantiation-dependent and the type isn't type-dependent,
> 	fall back to the normal processing.  Don't only call check_narrowing
> 	for scalar types.  Set CONSTRUCTOR_IS_DEPENDENT.
> 
> 	* g++.dg/cpp0x/Wnarrowing15.C: New test.
> 	* g++.dg/cpp0x/constexpr-decltype3.C: New test.
> 	* g++.dg/cpp1y/Wnarrowing1.C: New test.
> 
> diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
> index 5cc8f88d522..778874cccd6 100644
> --- gcc/cp/cp-tree.h
> +++ gcc/cp/cp-tree.h
> @@ -424,6 +424,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
>         DECL_FINAL_P (in FUNCTION_DECL)
>         QUALIFIED_NAME_IS_TEMPLATE (in SCOPE_REF)
>         DECLTYPE_FOR_INIT_CAPTURE (in DECLTYPE_TYPE)
> +      CONSTRUCTOR_IS_DEPENDENT (in CONSTRUCTOR)
>         TINFO_USED_TEMPLATE_ID (in TEMPLATE_INFO)
>         PACK_EXPANSION_SIZEOF_P (in *_PACK_EXPANSION)
>         OVL_USING_P (in OVERLOAD)
> @@ -4202,6 +4203,11 @@ more_aggr_init_expr_args_p (const aggr_init_expr_arg_iterator *iter)
>      B b{1,2}, not B b({1,2}) or B b = {1,2}.  */
>   #define CONSTRUCTOR_IS_DIRECT_INIT(NODE) (TREE_LANG_FLAG_0 (CONSTRUCTOR_CHECK (NODE)))
>   
> +/* True if this CONSTRUCTOR is instantiation-dependent and needs to be
> +   substituted.  */
> +#define CONSTRUCTOR_IS_DEPENDENT(NODE) \
> +  (TREE_LANG_FLAG_1 (CONSTRUCTOR_CHECK (NODE)))
> +
>   /* True if this CONSTRUCTOR should not be used as a variable initializer
>      because it was loaded from a constexpr variable with mutable fields.  */
>   #define CONSTRUCTOR_MUTABLE_POISON(NODE) \
> diff --git gcc/cp/pt.c gcc/cp/pt.c
> index e4f76478f54..ae77bae6b29 100644
> --- gcc/cp/pt.c
> +++ gcc/cp/pt.c
> @@ -25800,6 +25800,11 @@ instantiation_dependent_r (tree *tp, int *walk_subtrees,
>   	return *tp;
>         break;
>   
> +    case CONSTRUCTOR:
> +      if (CONSTRUCTOR_IS_DEPENDENT (*tp))
> +	return *tp;
> +      break;
> +
>       default:
>         break;
>       }
> diff --git gcc/cp/semantics.c gcc/cp/semantics.c
> index e654750d249..4ff09ad3fb7 100644
> --- gcc/cp/semantics.c
> +++ gcc/cp/semantics.c
> @@ -2795,11 +2795,14 @@ finish_compound_literal (tree type, tree compound_literal,
>   	  return error_mark_node;
>         }
>   
> -  if (processing_template_decl)
> +  if (instantiation_dependent_expression_p (compound_literal)
> +      || dependent_type_p (type))
>       {
>         TREE_TYPE (compound_literal) = type;
>         /* Mark the expression as a compound literal.  */
>         TREE_HAS_CONSTRUCTOR (compound_literal) = 1;
> +      /* And as instantiation-dependent.  */
> +      CONSTRUCTOR_IS_DEPENDENT (compound_literal) = true;
>         if (fcl_context == fcl_c99)
>   	CONSTRUCTOR_C99_COMPOUND_LITERAL (compound_literal) = 1;
>         return compound_literal;
> @@ -2822,8 +2825,7 @@ finish_compound_literal (tree type, tree compound_literal,
>         && check_array_initializer (NULL_TREE, type, compound_literal))
>       return error_mark_node;
>     compound_literal = reshape_init (type, compound_literal, complain);
> -  if (SCALAR_TYPE_P (type)
> -      && !BRACE_ENCLOSED_INITIALIZER_P (compound_literal)
> +  if (!BRACE_ENCLOSED_INITIALIZER_P (compound_literal)
>         && !check_narrowing (type, compound_literal, complain))
>       return error_mark_node;

Hmm, I wonder why we would want to call check_narrowing here at all; we 
ought to get the narrowing warning from the normal conversion process. 
Perhaps the issue is that we need to pass LOOKUP_NO_NARROWING to 
digest_init.

Jason

  reply	other threads:[~2019-01-21 20:15 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-17 19:09 Marek Polacek
2019-01-17 21:17 ` Jason Merrill
2019-01-18 14:12   ` Marek Polacek
2019-01-21 20:15     ` Jason Merrill [this message]
2019-01-22 21:12       ` Marek Polacek
2019-01-23 14:01         ` Jason Merrill
2019-01-23 18:29           ` Marek Polacek
2019-01-23 20:51             ` Jason Merrill
2019-01-25  0:51               ` Marek Polacek
2019-01-25 15:06                 ` Jason Merrill
2019-01-25 21:55                   ` Marek Polacek
2019-01-27  0:25                     ` Jason Merrill
2019-01-27 20:18                       ` Marek Polacek

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=bd67f9ca-138f-f42e-1ffd-5c4cf80893f3@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).