* [C++ Patch/RFC] Back to PR 53159
@ 2014-07-10 19:26 Paolo Carlini
2014-07-10 20:55 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Carlini @ 2014-07-10 19:26 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 371 bytes --]
Hi,
after more than 2 years, I'm finally back to this issue:
https://gcc.gnu.org/ml/gcc-patches/2012-05/msg01442.html
https://gcc.gnu.org/ml/gcc-patches/2012-05/msg01502.html
and the below draft, which passes testing, tries to implement as closely
as possible what Jason suggested in the thread above. How does it look?
Thanks!
Paolo.
///////////////////
[-- Attachment #2: patch_53159 --]
[-- Type: text/plain, Size: 3978 bytes --]
Index: cp/call.c
===================================================================
--- cp/call.c (revision 212431)
+++ cp/call.c (working copy)
@@ -3773,9 +3773,13 @@ build_user_type_conversion_1 (tree totype, tree ex
if (flags & LOOKUP_NO_NARROWING)
conv->check_narrowing = true;
- /* Combine it with the second conversion sequence. */
- cand->second_conv = merge_conversion_sequences (conv,
- cand->second_conv);
+ if (!(flags & LOOKUP_FOR_CHECK_NARROWING))
+ /* Combine it with the second conversion sequence. */
+ cand->second_conv = merge_conversion_sequences (conv,
+ cand->second_conv);
+ else
+ /* For convert_for_check_narrowing drop the second conversion. */
+ cand->second_conv = conv;
return cand;
}
@@ -3809,6 +3813,37 @@ build_user_type_conversion (tree totype, tree expr
return ret;
}
+/* Used by check_narrowing. */
+
+tree
+convert_for_check_narrowing (tree totype, tree expr)
+{
+ struct z_candidate *cand;
+ tree ret;
+
+ bool subtime = timevar_cond_start (TV_OVERLOAD);
+ cand = build_user_type_conversion_1 (totype, expr,
+ LOOKUP_NORMAL
+ | LOOKUP_FOR_CHECK_NARROWING,
+ tf_none);
+
+ if (cand)
+ {
+ if (cand->second_conv->kind == ck_ambig)
+ ret = error_mark_node;
+ else
+ ret = convert_like (cand->second_conv, expr, tf_none);
+ }
+ else
+ ret = NULL_TREE;
+
+ if (!ret || ret == error_mark_node)
+ ret = expr;
+
+ timevar_cond_stop (TV_OVERLOAD, subtime);
+ return ret;
+}
+
/* Subroutine of convert_nontype_argument.
EXPR is an argument for a template non-type parameter of integral or
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h (revision 212431)
+++ cp/cp-tree.h (working copy)
@@ -4573,6 +4573,8 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, T
#define LOOKUP_NO_NON_INTEGRAL (LOOKUP_NO_RVAL_BIND << 1)
/* Used for delegating constructors in order to diagnose self-delegation. */
#define LOOKUP_DELEGATING_CONS (LOOKUP_NO_NON_INTEGRAL << 1)
+/* Used by convert_for_check_narrowing. */
+#define LOOKUP_FOR_CHECK_NARROWING (LOOKUP_DELEGATING_CONS << 1)
#define LOOKUP_NAMESPACES_ONLY(F) \
(((F) & LOOKUP_PREFER_NAMESPACES) && !((F) & LOOKUP_PREFER_TYPES))
@@ -5050,6 +5052,8 @@ extern bool sufficient_parms_p (const_tree);
extern tree type_decays_to (tree);
extern tree build_user_type_conversion (tree, tree, int,
tsubst_flags_t);
+extern tree convert_for_check_narrowing (tree, tree);
+
extern tree build_new_function_call (tree, vec<tree, va_gc> **, bool,
tsubst_flags_t);
extern tree build_operator_new_call (tree, vec<tree, va_gc> **, tree *,
Index: cp/typeck2.c
===================================================================
--- cp/typeck2.c (revision 212431)
+++ cp/typeck2.c (working copy)
@@ -854,6 +854,13 @@ check_narrowing (tree type, tree init)
if (!warn_narrowing || !ARITHMETIC_TYPE_P (type))
return;
+ if (CLASS_TYPE_P (ftype))
+ {
+ /* Look through, eg, conversion operators (c++/53159). */
+ init = convert_for_check_narrowing (type, init);
+ ftype = unlowered_expr_type (init);
+ }
+
if (BRACE_ENCLOSED_INITIALIZER_P (init)
&& TREE_CODE (type) == COMPLEX_TYPE)
{
Index: testsuite/g++.dg/cpp0x/Wnarrowing1.C
===================================================================
--- testsuite/g++.dg/cpp0x/Wnarrowing1.C (revision 0)
+++ testsuite/g++.dg/cpp0x/Wnarrowing1.C (working copy)
@@ -0,0 +1,14 @@
+// PR c++/53159
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wnarrowing -Wno-overflow" }
+
+struct X
+{
+ constexpr operator int() { return __INT_MAX__; }
+};
+
+int f() { return __INT_MAX__; }
+
+signed char a { __INT_MAX__ }; // { dg-warning "narrowing conversion" }
+signed char b { f() }; // { dg-warning "narrowing conversion" }
+signed char c { X{} }; // { dg-warning "narrowing conversion" }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ Patch/RFC] Back to PR 53159
2014-07-10 19:26 [C++ Patch/RFC] Back to PR 53159 Paolo Carlini
@ 2014-07-10 20:55 ` Jason Merrill
2014-07-10 22:21 ` Paolo Carlini
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2014-07-10 20:55 UTC (permalink / raw)
To: Paolo Carlini, gcc-patches
Hmm, why aren't we already getting the error from
> if (convs->check_narrowing)
> check_narrowing (totype, expr);
in convert_like_real? Is it that we need to copy LOOKUP_NO_NARROWING
into convflags in build_user_type_conversion_1?
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ Patch/RFC] Back to PR 53159
2014-07-10 20:55 ` Jason Merrill
@ 2014-07-10 22:21 ` Paolo Carlini
2014-07-10 22:32 ` Paolo Carlini
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Carlini @ 2014-07-10 22:21 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 865 bytes --]
Hi,
On 07/10/2014 10:55 PM, Jason Merrill wrote:
> Hmm, why aren't we already getting the error from
>
>> if (convs->check_narrowing)
>> check_narrowing (totype, expr);
>
> in convert_like_real? Is it that we need to copy LOOKUP_NO_NARROWING
> into convflags in build_user_type_conversion_1?
Ah, ah, thanks. Frankly I noticed that flag but somehow decided to not
focus on it :(
Anyway, certainly build_user_type_conversion_1 can be tweaked as you are
suggesting, the only missing bit is that it doesn't get
LOOKUP_NO_NARROWING in the flags, and cannot figure it out from expr
because it's a TARGET_EXPR at that point. Thus it seems to me we have to
pass it down from, say, check_initializer when init is still a
CONSTRUCTOR and its BRACE_ENCLOSED_INITIALIZER_P (init) is set? I'm
going to test the below... makes sense?
Paolo.
//////////////////
[-- Attachment #2: p --]
[-- Type: text/plain, Size: 925 bytes --]
Index: call.c
===================================================================
--- call.c (revision 212431)
+++ call.c (working copy)
@@ -3586,7 +3586,8 @@ build_user_type_conversion_1 (tree totype, tree ex
/* It's OK to bind a temporary for converting constructor arguments, but
not in converting the return value of a conversion operator. */
- convflags = ((flags & LOOKUP_NO_TEMP_BIND) | LOOKUP_NO_CONVERSION);
+ convflags = ((flags & LOOKUP_NO_TEMP_BIND) | LOOKUP_NO_CONVERSION
+ | (flags & LOOKUP_NO_NARROWING));
flags &= ~LOOKUP_NO_TEMP_BIND;
if (ctors)
Index: decl.c
===================================================================
--- decl.c (revision 212431)
+++ decl.c (working copy)
@@ -5707,6 +5707,7 @@ check_initializer (tree decl, tree init, int flags
return NULL_TREE;
}
}
+ flags |= LOOKUP_NO_NARROWING;
}
if (TREE_CODE (decl) == CONST_DECL)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ Patch/RFC] Back to PR 53159
2014-07-10 22:21 ` Paolo Carlini
@ 2014-07-10 22:32 ` Paolo Carlini
2014-07-10 22:43 ` Paolo Carlini
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Carlini @ 2014-07-10 22:32 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 95 bytes --]
... the below is another, very safe, option for setting
LOOKUP_NO_NARROWING in flags.
Paolo.
[-- Attachment #2: p2 --]
[-- Type: text/plain, Size: 995 bytes --]
Index: call.c
===================================================================
--- call.c (revision 212431)
+++ call.c (working copy)
@@ -3586,7 +3586,8 @@ build_user_type_conversion_1 (tree totype, tree ex
/* It's OK to bind a temporary for converting constructor arguments, but
not in converting the return value of a conversion operator. */
- convflags = ((flags & LOOKUP_NO_TEMP_BIND) | LOOKUP_NO_CONVERSION);
+ convflags = ((flags & LOOKUP_NO_TEMP_BIND) | LOOKUP_NO_CONVERSION
+ | (flags & LOOKUP_NO_NARROWING));
flags &= ~LOOKUP_NO_TEMP_BIND;
if (ctors)
Index: decl.c
===================================================================
--- decl.c (revision 212431)
+++ decl.c (working copy)
@@ -5755,6 +5755,7 @@ check_initializer (tree decl, tree init, int flags
}
else
{
+ flags |= LOOKUP_NO_NARROWING;
init = reshape_init (type, init, tf_warning_or_error);
if (SCALAR_TYPE_P (type))
check_narrowing (type, init);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ Patch/RFC] Back to PR 53159
2014-07-10 22:32 ` Paolo Carlini
@ 2014-07-10 22:43 ` Paolo Carlini
2014-07-11 17:53 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Carlini @ 2014-07-10 22:43 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 316 bytes --]
... and of course the problem with all such ideas is that we easily end
up warning twice in all the simple cases which check_narrowing can
already handle. Something like the attached has more chances of passing
the testsuite while not regressing in terms of duplicate warnings (which
are hard to notice)
Paolo.
[-- Attachment #2: p3 --]
[-- Type: text/plain, Size: 1136 bytes --]
Index: call.c
===================================================================
--- call.c (revision 212431)
+++ call.c (working copy)
@@ -3586,7 +3586,8 @@ build_user_type_conversion_1 (tree totype, tree ex
/* It's OK to bind a temporary for converting constructor arguments, but
not in converting the return value of a conversion operator. */
- convflags = ((flags & LOOKUP_NO_TEMP_BIND) | LOOKUP_NO_CONVERSION);
+ convflags = ((flags & LOOKUP_NO_TEMP_BIND) | LOOKUP_NO_CONVERSION
+ | (flags & LOOKUP_NO_NARROWING));
flags &= ~LOOKUP_NO_TEMP_BIND;
if (ctors)
Index: decl.c
===================================================================
--- decl.c (revision 212431)
+++ decl.c (working copy)
@@ -5757,7 +5757,12 @@ check_initializer (tree decl, tree init, int flags
{
init = reshape_init (type, init, tf_warning_or_error);
if (SCALAR_TYPE_P (type))
- check_narrowing (type, init);
+ {
+ if (!CLASS_TYPE_P (TREE_TYPE (init)))
+ check_narrowing (type, init);
+ else
+ flags |= LOOKUP_NO_NARROWING;
+ }
}
}
else if (TREE_CODE (init) == TREE_LIST
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ Patch/RFC] Back to PR 53159
2014-07-10 22:43 ` Paolo Carlini
@ 2014-07-11 17:53 ` Jason Merrill
0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2014-07-11 17:53 UTC (permalink / raw)
To: Paolo Carlini, gcc-patches
On 07/10/2014 06:38 PM, Paolo Carlini wrote:
> if (SCALAR_TYPE_P (type))
> + if (!CLASS_TYPE_P (TREE_TYPE (init)))
Why this change? check_narrowing only deals with scalar types.
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-11 17:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-10 19:26 [C++ Patch/RFC] Back to PR 53159 Paolo Carlini
2014-07-10 20:55 ` Jason Merrill
2014-07-10 22:21 ` Paolo Carlini
2014-07-10 22:32 ` Paolo Carlini
2014-07-10 22:43 ` Paolo Carlini
2014-07-11 17:53 ` Jason Merrill
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).