| Message ID | 20121122120654.GA217@x4 |
|---|---|
| State | New |
| Headers | show |
Hi,
On 11/22/2012 01:06 PM, Markus Trippelsdorf wrote:
> The fix is trivial. I would appreciate if someone could commit this.
I agree it qualifies as obvious and I mean to commit the patch on your
behalf before the end of the day if nobody objects. To be super-safe, I
guess I will also run the testsuite with the current code and trivial_p
initialized to true at the beginning of the function.
Thanks!
Paolo.
On Thu, Nov 22, 2012 at 02:28:18PM +0100, Paolo Carlini wrote: > On 11/22/2012 01:06 PM, Markus Trippelsdorf wrote: > >The fix is trivial. I would appreciate if someone could commit this. > I agree it qualifies as obvious and I mean to commit the patch on > your behalf before the end of the day if nobody objects. To be > super-safe, I guess I will also run the testsuite with the current > code and trivial_p initialized to true at the beginning of the > function. I think trivial_p used to be uninitialized only for deleted_p functions, not sure if those are considered trivial or not (and whether it matters)... Jakub
Hi, On 11/22/2012 04:06 PM, Jakub Jelinek wrote: > On Thu, Nov 22, 2012 at 02:28:18PM +0100, Paolo Carlini wrote: >> On 11/22/2012 01:06 PM, Markus Trippelsdorf wrote: >>> The fix is trivial. I would appreciate if someone could commit this. >> I agree it qualifies as obvious and I mean to commit the patch on >> your behalf before the end of the day if nobody objects. To be >> super-safe, I guess I will also run the testsuite with the current >> code and trivial_p initialized to true at the beginning of the >> function. > I think trivial_p used to be uninitialized only for deleted_p functions, > not sure if those are considered trivial or not (and whether it matters)... I think that the Valgrind complain is benign and, by the way, I just run the testsuite with current code + trivial_p initialized to true with no regressions. In detail what is happening is that, as you say, in the special case of deleted_p true and lambdas, we return from synthesized_method_walk and trivial_p is still uninitialized. Then we have: if (!trivial_p && type_has_trivial_fn (type, kind)) type_set_nontrivial_flag (type, kind); which, if trivial_p is is still uninitialized may not do its job of forcing nontrivial set in type: I don't think this is a serious problem for lambdas (in any case, when code elsewhere detects deleted shouldn't bother anyway of checking triviality; testsuite passes with trivial_p initialized to true too), but Markus (and me ;) proposes anyway to initialize trivial_p to false, thus we should be fine (way below trivial_p is used again but only when deleted_p is false). Paolo.
On 11/22/2012 11:03 AM, Paolo Carlini wrote: > initialized to true too), but Markus (and me ;) proposes anyway to > initialize trivial_p to false, thus we should be fine (way below > trivial_p is used again but only when deleted_p is false). It certainly can't hurt. Jason
On 11/23/2012 10:46 AM, Jason Merrill wrote: > On 11/22/2012 11:03 AM, Paolo Carlini wrote: >> initialized to true too), but Markus (and me ;) proposes anyway to >> initialize trivial_p to false, thus we should be fine (way below >> trivial_p is used again but only when deleted_p is false). > > It certainly can't hurt. Actually...we should be calculating triviality normally regardless of whether the function is deleted. Why isn't that happening? Jason
Hi, On 11/23/2012 04:47 PM, Jason Merrill wrote: > On 11/23/2012 10:46 AM, Jason Merrill wrote: >> On 11/22/2012 11:03 AM, Paolo Carlini wrote: >>> initialized to true too), but Markus (and me ;) proposes anyway to >>> initialize trivial_p to false, thus we should be fine (way below >>> trivial_p is used again but only when deleted_p is false). >> >> It certainly can't hurt. > > Actually...we should be calculating triviality normally regardless of > whether the function is deleted. Why isn't that happening? Because in synthesized_method_walk for lambdas we have this early return: if (deleted_p) { /* "The closure type associated with a lambda-expression has a deleted default constructor and a deleted copy assignment operator." This is diagnosed in maybe_explain_implicit_delete. */ if (LAMBDA_TYPE_P (ctype) && (sfk == sfk_constructor || sfk == sfk_copy_assignment)) { *deleted_p = true; return; } *deleted_p = false; } Paolo.
diff --git a/gcc/cp/method.c b/gcc/cp/method.c index 6dcb63a..778daa8 100644 --- a/gcc/cp/method.c +++ b/gcc/cp/method.c @@ -1518,7 +1518,6 @@ implicitly_declare_fn (special_function_kind kind, tree type, tree name; HOST_WIDE_INT saved_processing_template_decl; bool deleted_p; - bool trivial_p; bool constexpr_p; /* Because we create declarations for implicitly declared functions @@ -1597,12 +1596,13 @@ implicitly_declare_fn (special_function_kind kind, tree type, tree inherited_base = (inherited_ctor ? DECL_CONTEXT (inherited_ctor) : NULL_TREE); + bool trivial_p = false; + if (inherited_ctor && TREE_CODE (inherited_ctor) == TEMPLATE_DECL) { /* For an inheriting constructor template, just copy these flags from the inherited constructor template for now. */ raises = TYPE_RAISES_EXCEPTIONS (TREE_TYPE (inherited_ctor)); - trivial_p = false; deleted_p = DECL_DELETED_FN (DECL_TEMPLATE_RESULT (inherited_ctor)); constexpr_p = DECL_DECLARED_CONSTEXPR_P (DECL_TEMPLATE_RESULT (inherited_ctor));