Message ID | 20190117190921.GM19569@redhat.com |
---|---|
State | New |
Headers | show |
Series | C++ PATCH for c++/78244 - narrowing conversion in template not detected, part 2 | expand |
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. 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. Jason
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; if (TREE_CODE (type) == ARRAY_TYPE diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing15.C gcc/testsuite/g++.dg/cpp0x/Wnarrowing15.C new file mode 100644 index 00000000000..4e7c17dcfca --- /dev/null +++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing15.C @@ -0,0 +1,14 @@ +// PR c++/78244 +// { dg-do compile { target c++11 } } + +template <typename T> +auto f1() -> decltype(int{2.0}, void()) { } // { dg-error "narrowing conversion" } + +template <typename T> +auto f2() -> decltype(int{2.0}) { return 1; } // { dg-error "narrowing conversion" } + +template <typename T> +auto f3() -> decltype(void(), int{2.0}) { return 1; } // { dg-error "narrowing conversion" } + +template <typename T> +auto f4() -> decltype((int{2.0})) { return 1; } // { dg-error "narrowing conversion" } diff --git gcc/testsuite/g++.dg/cpp0x/constexpr-decltype3.C gcc/testsuite/g++.dg/cpp0x/constexpr-decltype3.C new file mode 100644 index 00000000000..fd05366de50 --- /dev/null +++ gcc/testsuite/g++.dg/cpp0x/constexpr-decltype3.C @@ -0,0 +1,25 @@ +// PR c++/88815 +// { dg-do compile { target c++11 } } + +struct true_type { + constexpr operator bool() const { return true; } +}; + +struct false_type { + constexpr operator bool() const { return false; } +}; + +template<int (*p)()> +true_type is_constexpr_impl(decltype(int{(p(), 0U)})); + +template<int (*p)()> +false_type is_constexpr_impl(...); + +template<int (*p)()> +using is_constexpr = decltype(is_constexpr_impl<p>(0)); + +constexpr int f() { return 0; } +int g() { return 0; } + +static_assert(is_constexpr<f>(), ""); +static_assert(!is_constexpr<g>(), ""); diff --git gcc/testsuite/g++.dg/cpp1y/Wnarrowing1.C gcc/testsuite/g++.dg/cpp1y/Wnarrowing1.C new file mode 100644 index 00000000000..e1e499542f0 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/Wnarrowing1.C @@ -0,0 +1,5 @@ +// PR c++/78244 +// { dg-do compile { target c++14 } } + +template<typename> +decltype(int{1.1}) v; // { dg-error "narrowing conversion" }
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
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. Marek
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
On Wed, Jan 23, 2019 at 09:00:36AM -0500, Jason Merrill wrote: > I was talking about digest_init, not reshape_init. digest_init calls > convert_for_initialization. /facepalm So yes, digest_init calls convert_for_initialization which will end up calling perform_implicit_conversion_flags which could call convert_like_real where the narrowing warnings are given, but it doesn't, we go to this case: else if (processing_template_decl && conv->kind != ck_identity) { /* In a template, we are only concerned about determining the type of non-dependent expressions, so we do not have to perform the actual conversion. But for initializers, we need to be able to perform it at instantiation (or instantiate_non_dependent_expr) time. */ expr = build1 (IMPLICIT_CONV_EXPR, type, expr); finish_decltype_type throws away the expression because it's not dependent, and only uses its type. So narrowing remains undetected. Not sure if I should mess with perform_implicit_conversion_flags.
On Wed, Jan 23, 2019 at 12:57 PM Marek Polacek <polacek@redhat.com> wrote: > > On Wed, Jan 23, 2019 at 09:00:36AM -0500, Jason Merrill wrote: > > I was talking about digest_init, not reshape_init. digest_init calls > > convert_for_initialization. > > /facepalm > > So yes, digest_init calls convert_for_initialization which will end up > calling perform_implicit_conversion_flags which could call convert_like_real > where the narrowing warnings are given, but it doesn't, we go to this case: > > else if (processing_template_decl && conv->kind != ck_identity) > { > /* In a template, we are only concerned about determining the > type of non-dependent expressions, so we do not have to > perform the actual conversion. But for initializers, we > need to be able to perform it at instantiation > (or instantiate_non_dependent_expr) time. */ > expr = build1 (IMPLICIT_CONV_EXPR, type, expr); > > finish_decltype_type throws away the expression because it's not dependent, and > only uses its type. So narrowing remains undetected. Not sure if I should mess > with perform_implicit_conversion_flags. Let's try that; this is a situation where the comment is incorrect. Perhaps just call check_narrowing here if appropriate, rather than go through the whole conversion machinery. Jason
On Wed, Jan 23, 2019 at 03:34:04PM -0500, Jason Merrill wrote: > On Wed, Jan 23, 2019 at 12:57 PM Marek Polacek <polacek@redhat.com> wrote: > > > > On Wed, Jan 23, 2019 at 09:00:36AM -0500, Jason Merrill wrote: > > > I was talking about digest_init, not reshape_init. digest_init calls > > > convert_for_initialization. > > > > /facepalm > > > > So yes, digest_init calls convert_for_initialization which will end up > > calling perform_implicit_conversion_flags which could call convert_like_real > > where the narrowing warnings are given, but it doesn't, we go to this case: > > > > else if (processing_template_decl && conv->kind != ck_identity) > > { > > /* In a template, we are only concerned about determining the > > type of non-dependent expressions, so we do not have to > > perform the actual conversion. But for initializers, we > > need to be able to perform it at instantiation > > (or instantiate_non_dependent_expr) time. */ > > expr = build1 (IMPLICIT_CONV_EXPR, type, expr); > > > > finish_decltype_type throws away the expression because it's not dependent, and > > only uses its type. So narrowing remains undetected. Not sure if I should mess > > with perform_implicit_conversion_flags. > > Let's try that; this is a situation where the comment is incorrect. > Perhaps just call check_narrowing here if appropriate, rather than go > through the whole conversion machinery. I have not been successful. First, I modified perform_implicit_conversion_flags to go the convert_like route when dealing with something non-dependent. That breaks e.g. in build_value_init: 346 /* The AGGR_INIT_EXPR tweaking below breaks in templates. */ 347 gcc_assert (!processing_template_decl 348 || (SCALAR_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)); Even if I restrict the convert_like way for non-dependent exprs in a template for scalars, it still breaks elsewhere, e.g. constexpr-template3.C where it complains about taking the address of an rvalue. Second, I added check_narrowing to the processing_template_decl case in perform_implicit_conversion_flags. That works except it breaks constexpr-inst1.C -- we no longer get the error. That's because currently check_narrowing in finish_compound_literal calls maybe_constant_init, which calls instantiate_constexpr_fns and we get the desired diagnostic. But if I move check_narrowing to perform_implicit_conversion_flags, we no longer call it in this case -- processing_template_decl is 0 so we call convert_like but that doesn't do the trick. So, back to the patch that leaves check_narrowing in finish_compound_literal? Marek
On 1/24/19 7:17 PM, Marek Polacek wrote: > On Wed, Jan 23, 2019 at 03:34:04PM -0500, Jason Merrill wrote: >> On Wed, Jan 23, 2019 at 12:57 PM Marek Polacek <polacek@redhat.com> wrote: >>> >>> On Wed, Jan 23, 2019 at 09:00:36AM -0500, Jason Merrill wrote: >>>> I was talking about digest_init, not reshape_init. digest_init calls >>>> convert_for_initialization. >>> >>> /facepalm >>> >>> So yes, digest_init calls convert_for_initialization which will end up >>> calling perform_implicit_conversion_flags which could call convert_like_real >>> where the narrowing warnings are given, but it doesn't, we go to this case: >>> >>> else if (processing_template_decl && conv->kind != ck_identity) >>> { >>> /* In a template, we are only concerned about determining the >>> type of non-dependent expressions, so we do not have to >>> perform the actual conversion. But for initializers, we >>> need to be able to perform it at instantiation >>> (or instantiate_non_dependent_expr) time. */ >>> expr = build1 (IMPLICIT_CONV_EXPR, type, expr); >>> >>> finish_decltype_type throws away the expression because it's not dependent, and >>> only uses its type. So narrowing remains undetected. Not sure if I should mess >>> with perform_implicit_conversion_flags. >> >> Let's try that; this is a situation where the comment is incorrect. >> Perhaps just call check_narrowing here if appropriate, rather than go >> through the whole conversion machinery. > > I have not been successful. > > First, I modified perform_implicit_conversion_flags to go the convert_like > route when dealing with something non-dependent. That breaks e.g. in > build_value_init: > 346 /* The AGGR_INIT_EXPR tweaking below breaks in templates. */ > 347 gcc_assert (!processing_template_decl > 348 || (SCALAR_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)); > Even if I restrict the convert_like way for non-dependent exprs in a template > for scalars, it still breaks elsewhere, e.g. constexpr-template3.C where it > complains about taking the address of an rvalue. > > Second, I added check_narrowing to the processing_template_decl case in > perform_implicit_conversion_flags. That works except it breaks > constexpr-inst1.C -- we no longer get the error. That's because currently > check_narrowing in finish_compound_literal calls maybe_constant_init, which > calls instantiate_constexpr_fns and we get the desired diagnostic. But if > I move check_narrowing to perform_implicit_conversion_flags, we no longer > call it in this case -- processing_template_decl is 0 so we call convert_like > but that doesn't do the trick. > > So, back to the patch that leaves check_narrowing in finish_compound_literal? That patch still needs a test for the aggregate case. Jason
On Fri, Jan 25, 2019 at 10:05:00AM -0500, Jason Merrill wrote: > On 1/24/19 7:17 PM, Marek Polacek wrote: > > On Wed, Jan 23, 2019 at 03:34:04PM -0500, Jason Merrill wrote: > > > On Wed, Jan 23, 2019 at 12:57 PM Marek Polacek <polacek@redhat.com> wrote: > > > > > > > > On Wed, Jan 23, 2019 at 09:00:36AM -0500, Jason Merrill wrote: > > > > > I was talking about digest_init, not reshape_init. digest_init calls > > > > > convert_for_initialization. > > > > > > > > /facepalm > > > > > > > > So yes, digest_init calls convert_for_initialization which will end up > > > > calling perform_implicit_conversion_flags which could call convert_like_real > > > > where the narrowing warnings are given, but it doesn't, we go to this case: > > > > > > > > else if (processing_template_decl && conv->kind != ck_identity) > > > > { > > > > /* In a template, we are only concerned about determining the > > > > type of non-dependent expressions, so we do not have to > > > > perform the actual conversion. But for initializers, we > > > > need to be able to perform it at instantiation > > > > (or instantiate_non_dependent_expr) time. */ > > > > expr = build1 (IMPLICIT_CONV_EXPR, type, expr); > > > > > > > > finish_decltype_type throws away the expression because it's not dependent, and > > > > only uses its type. So narrowing remains undetected. Not sure if I should mess > > > > with perform_implicit_conversion_flags. > > > > > > Let's try that; this is a situation where the comment is incorrect. > > > Perhaps just call check_narrowing here if appropriate, rather than go > > > through the whole conversion machinery. > > > > I have not been successful. > > > > First, I modified perform_implicit_conversion_flags to go the convert_like > > route when dealing with something non-dependent. That breaks e.g. in > > build_value_init: > > 346 /* The AGGR_INIT_EXPR tweaking below breaks in templates. */ > > 347 gcc_assert (!processing_template_decl > > 348 || (SCALAR_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)); > > Even if I restrict the convert_like way for non-dependent exprs in a template > > for scalars, it still breaks elsewhere, e.g. constexpr-template3.C where it > > complains about taking the address of an rvalue. > > > > Second, I added check_narrowing to the processing_template_decl case in > > perform_implicit_conversion_flags. That works except it breaks > > constexpr-inst1.C -- we no longer get the error. That's because currently > > check_narrowing in finish_compound_literal calls maybe_constant_init, which > > calls instantiate_constexpr_fns and we get the desired diagnostic. But if > > I move check_narrowing to perform_implicit_conversion_flags, we no longer > > call it in this case -- processing_template_decl is 0 so we call convert_like > > but that doesn't do the trick. > > > > So, back to the patch that leaves check_narrowing in finish_compound_literal? > > That patch still needs a test for the aggregate case. Ok, this is a version with Wnarrowing16.C added. ...but we still don't warn for the TYPE_NON_AGGREGATE_CLASS case in finish_compound_literal, so the nightmare continues. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-01-25 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/Wnarrowing16.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 cd902ce1cf6..77e1425b435 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) @@ -4205,6 +4206,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 48c180cc13b..f8b3054533e 100644 --- gcc/cp/pt.c +++ gcc/cp/pt.c @@ -25815,6 +25815,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 72191395b47..f1df2561258 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; if (TREE_CODE (type) == ARRAY_TYPE diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing15.C gcc/testsuite/g++.dg/cpp0x/Wnarrowing15.C new file mode 100644 index 00000000000..4e7c17dcfca --- /dev/null +++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing15.C @@ -0,0 +1,14 @@ +// PR c++/78244 +// { dg-do compile { target c++11 } } + +template <typename T> +auto f1() -> decltype(int{2.0}, void()) { } // { dg-error "narrowing conversion" } + +template <typename T> +auto f2() -> decltype(int{2.0}) { return 1; } // { dg-error "narrowing conversion" } + +template <typename T> +auto f3() -> decltype(void(), int{2.0}) { return 1; } // { dg-error "narrowing conversion" } + +template <typename T> +auto f4() -> decltype((int{2.0})) { return 1; } // { dg-error "narrowing conversion" } diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing16.C gcc/testsuite/g++.dg/cpp0x/Wnarrowing16.C new file mode 100644 index 00000000000..21394be1b97 --- /dev/null +++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing16.C @@ -0,0 +1,16 @@ +// PR c++/78244 +// { dg-do compile { target c++11 } } + +struct S { int d; }; + +template <typename T> +auto f1() -> decltype(S{2.0}, void()) { } // { dg-error "narrowing conversion" } + +template <typename T> +auto f2() -> decltype(S{2.0}, 1) { return 1; } // { dg-error "narrowing conversion" } + +template <typename T> +auto f3() -> decltype(void(), S{2.0}, 1) { return 1; } // { dg-error "narrowing conversion" } + +template <typename T> +auto f4() -> decltype((S{2.0}, 1)) { return 1; } // { dg-error "narrowing conversion" } diff --git gcc/testsuite/g++.dg/cpp0x/constexpr-decltype3.C gcc/testsuite/g++.dg/cpp0x/constexpr-decltype3.C new file mode 100644 index 00000000000..fd05366de50 --- /dev/null +++ gcc/testsuite/g++.dg/cpp0x/constexpr-decltype3.C @@ -0,0 +1,25 @@ +// PR c++/88815 +// { dg-do compile { target c++11 } } + +struct true_type { + constexpr operator bool() const { return true; } +}; + +struct false_type { + constexpr operator bool() const { return false; } +}; + +template<int (*p)()> +true_type is_constexpr_impl(decltype(int{(p(), 0U)})); + +template<int (*p)()> +false_type is_constexpr_impl(...); + +template<int (*p)()> +using is_constexpr = decltype(is_constexpr_impl<p>(0)); + +constexpr int f() { return 0; } +int g() { return 0; } + +static_assert(is_constexpr<f>(), ""); +static_assert(!is_constexpr<g>(), ""); diff --git gcc/testsuite/g++.dg/cpp1y/Wnarrowing1.C gcc/testsuite/g++.dg/cpp1y/Wnarrowing1.C new file mode 100644 index 00000000000..e1e499542f0 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/Wnarrowing1.C @@ -0,0 +1,5 @@ +// PR c++/78244 +// { dg-do compile { target c++14 } } + +template<typename> +decltype(int{1.1}) v; // { dg-error "narrowing conversion" }
On Fri, Jan 25, 2019 at 4:20 PM Marek Polacek <polacek@redhat.com> wrote: > On Fri, Jan 25, 2019 at 10:05:00AM -0500, Jason Merrill wrote: > > On 1/24/19 7:17 PM, Marek Polacek wrote: > > > On Wed, Jan 23, 2019 at 03:34:04PM -0500, Jason Merrill wrote: > > > > On Wed, Jan 23, 2019 at 12:57 PM Marek Polacek <polacek@redhat.com> wrote: > > > > > > > > > > On Wed, Jan 23, 2019 at 09:00:36AM -0500, Jason Merrill wrote: > > > > > > I was talking about digest_init, not reshape_init. digest_init calls > > > > > > convert_for_initialization. > > > > > > > > > > /facepalm > > > > > > > > > > So yes, digest_init calls convert_for_initialization which will end up > > > > > calling perform_implicit_conversion_flags which could call convert_like_real > > > > > where the narrowing warnings are given, but it doesn't, we go to this case: > > > > > > > > > > else if (processing_template_decl && conv->kind != ck_identity) > > > > > { > > > > > /* In a template, we are only concerned about determining the > > > > > type of non-dependent expressions, so we do not have to > > > > > perform the actual conversion. But for initializers, we > > > > > need to be able to perform it at instantiation > > > > > (or instantiate_non_dependent_expr) time. */ > > > > > expr = build1 (IMPLICIT_CONV_EXPR, type, expr); > > > > > > > > > > finish_decltype_type throws away the expression because it's not dependent, and > > > > > only uses its type. So narrowing remains undetected. Not sure if I should mess > > > > > with perform_implicit_conversion_flags. > > > > > > > > Let's try that; this is a situation where the comment is incorrect. > > > > Perhaps just call check_narrowing here if appropriate, rather than go > > > > through the whole conversion machinery. > > > > > > I have not been successful. > > > > > > First, I modified perform_implicit_conversion_flags to go the convert_like > > > route when dealing with something non-dependent. That breaks e.g. in > > > build_value_init: > > > 346 /* The AGGR_INIT_EXPR tweaking below breaks in templates. */ > > > 347 gcc_assert (!processing_template_decl > > > 348 || (SCALAR_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)); > > > Even if I restrict the convert_like way for non-dependent exprs in a template > > > for scalars, it still breaks elsewhere, e.g. constexpr-template3.C where it > > > complains about taking the address of an rvalue. > > > > > > Second, I added check_narrowing to the processing_template_decl case in > > > perform_implicit_conversion_flags. That works except it breaks > > > constexpr-inst1.C -- we no longer get the error. That's because currently > > > check_narrowing in finish_compound_literal calls maybe_constant_init, which > > > calls instantiate_constexpr_fns and we get the desired diagnostic. But if > > > I move check_narrowing to perform_implicit_conversion_flags, we no longer > > > call it in this case -- processing_template_decl is 0 so we call convert_like > > > but that doesn't do the trick. > > > > > > So, back to the patch that leaves check_narrowing in finish_compound_literal? > > > > That patch still needs a test for the aggregate case. > > Ok, this is a version with Wnarrowing16.C added. > > ...but we still don't warn for the TYPE_NON_AGGREGATE_CLASS case in > finish_compound_literal, so the nightmare continues. Alas. Are you going to keep looking at that, or would you like me to take over? > - 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; Does this hunk actually make a difference? It looks like check_narrowing only does anything for arithmetic types. OK with or without this hunk. Jason
On Sat, Jan 26, 2019 at 07:22:17PM -0500, Jason Merrill wrote: > On Fri, Jan 25, 2019 at 4:20 PM Marek Polacek <polacek@redhat.com> wrote: > > On Fri, Jan 25, 2019 at 10:05:00AM -0500, Jason Merrill wrote: > > > On 1/24/19 7:17 PM, Marek Polacek wrote: > > > > On Wed, Jan 23, 2019 at 03:34:04PM -0500, Jason Merrill wrote: > > > > > On Wed, Jan 23, 2019 at 12:57 PM Marek Polacek <polacek@redhat.com> wrote: > > > > > > > > > > > > On Wed, Jan 23, 2019 at 09:00:36AM -0500, Jason Merrill wrote: > > > > > > > I was talking about digest_init, not reshape_init. digest_init calls > > > > > > > convert_for_initialization. > > > > > > > > > > > > /facepalm > > > > > > > > > > > > So yes, digest_init calls convert_for_initialization which will end up > > > > > > calling perform_implicit_conversion_flags which could call convert_like_real > > > > > > where the narrowing warnings are given, but it doesn't, we go to this case: > > > > > > > > > > > > else if (processing_template_decl && conv->kind != ck_identity) > > > > > > { > > > > > > /* In a template, we are only concerned about determining the > > > > > > type of non-dependent expressions, so we do not have to > > > > > > perform the actual conversion. But for initializers, we > > > > > > need to be able to perform it at instantiation > > > > > > (or instantiate_non_dependent_expr) time. */ > > > > > > expr = build1 (IMPLICIT_CONV_EXPR, type, expr); > > > > > > > > > > > > finish_decltype_type throws away the expression because it's not dependent, and > > > > > > only uses its type. So narrowing remains undetected. Not sure if I should mess > > > > > > with perform_implicit_conversion_flags. > > > > > > > > > > Let's try that; this is a situation where the comment is incorrect. > > > > > Perhaps just call check_narrowing here if appropriate, rather than go > > > > > through the whole conversion machinery. > > > > > > > > I have not been successful. > > > > > > > > First, I modified perform_implicit_conversion_flags to go the convert_like > > > > route when dealing with something non-dependent. That breaks e.g. in > > > > build_value_init: > > > > 346 /* The AGGR_INIT_EXPR tweaking below breaks in templates. */ > > > > 347 gcc_assert (!processing_template_decl > > > > 348 || (SCALAR_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)); > > > > Even if I restrict the convert_like way for non-dependent exprs in a template > > > > for scalars, it still breaks elsewhere, e.g. constexpr-template3.C where it > > > > complains about taking the address of an rvalue. > > > > > > > > Second, I added check_narrowing to the processing_template_decl case in > > > > perform_implicit_conversion_flags. That works except it breaks > > > > constexpr-inst1.C -- we no longer get the error. That's because currently > > > > check_narrowing in finish_compound_literal calls maybe_constant_init, which > > > > calls instantiate_constexpr_fns and we get the desired diagnostic. But if > > > > I move check_narrowing to perform_implicit_conversion_flags, we no longer > > > > call it in this case -- processing_template_decl is 0 so we call convert_like > > > > but that doesn't do the trick. > > > > > > > > So, back to the patch that leaves check_narrowing in finish_compound_literal? > > > > > > That patch still needs a test for the aggregate case. > > > > Ok, this is a version with Wnarrowing16.C added. > > > > ...but we still don't warn for the TYPE_NON_AGGREGATE_CLASS case in > > finish_compound_literal, so the nightmare continues. > > Alas. Are you going to keep looking at that, or would you like me to take over? I think I should keep poking at it, but I welcome any advice. But perhaps we should leave this for GCC 10? I'll add the new test to the PR. > > - 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; > > Does this hunk actually make a difference? It looks like > check_narrowing only does anything for arithmetic types. No difference (I ran the testsuite with some additional checking). > OK with or without this hunk. Thanks for the reviews. Marek
diff --git gcc/cp/semantics.c gcc/cp/semantics.c index bc9d53800f7..828f1578697 100644 --- gcc/cp/semantics.c +++ gcc/cp/semantics.c @@ -2797,6 +2797,14 @@ finish_compound_literal (tree type, tree compound_literal, if (processing_template_decl) { + /* If there are no dependent expressions, we can detect narrowing + conversions. */ + if (SCALAR_TYPE_P (type) + && CONSTRUCTOR_NELTS (compound_literal) == 1 + && !check_narrowing (type, + CONSTRUCTOR_ELT (compound_literal, 0)->value, + complain)) + return error_mark_node; TREE_TYPE (compound_literal) = type; /* Mark the expression as a compound literal. */ TREE_HAS_CONSTRUCTOR (compound_literal) = 1; diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing15.C gcc/testsuite/g++.dg/cpp0x/Wnarrowing15.C new file mode 100644 index 00000000000..4e7c17dcfca --- /dev/null +++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing15.C @@ -0,0 +1,14 @@ +// PR c++/78244 +// { dg-do compile { target c++11 } } + +template <typename T> +auto f1() -> decltype(int{2.0}, void()) { } // { dg-error "narrowing conversion" } + +template <typename T> +auto f2() -> decltype(int{2.0}) { return 1; } // { dg-error "narrowing conversion" } + +template <typename T> +auto f3() -> decltype(void(), int{2.0}) { return 1; } // { dg-error "narrowing conversion" } + +template <typename T> +auto f4() -> decltype((int{2.0})) { return 1; } // { dg-error "narrowing conversion" } diff --git gcc/testsuite/g++.dg/cpp1y/Wnarrowing1.C gcc/testsuite/g++.dg/cpp1y/Wnarrowing1.C new file mode 100644 index 00000000000..e1e499542f0 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/Wnarrowing1.C @@ -0,0 +1,5 @@ +// PR c++/78244 +// { dg-do compile { target c++14 } } + +template<typename> +decltype(int{1.1}) v; // { dg-error "narrowing conversion" }