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: Wed, 23 Jan 2019 14:01:00 -0000	[thread overview]
Message-ID: <33c9ff91-1aff-d328-7349-767b196eaecf@redhat.com> (raw)
In-Reply-To: <20190122211021.GC26714@redhat.com>

On 1/22/19 4:10 PM, Marek Polacek wrote:
> On Mon, Jan 21, 2019 at 03:14:53PM -0500, Jason Merrill wrote:
>> 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.
> 
> reshape_init never ends up calling convert_for_assignment or convert_like_real,
> so even if we signalled to check narrowing to it, it wouldn't help.
> reshape_init calls check_narrowing just for enum initialization.

I was talking about digest_init, not reshape_init.  digest_init calls 
convert_for_initialization.

Jason

  reply	other threads:[~2019-01-23 14:00 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
2019-01-22 21:12       ` Marek Polacek
2019-01-23 14:01         ` Jason Merrill [this message]
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=33c9ff91-1aff-d328-7349-767b196eaecf@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).