| Message ID | Zg80hQRRH_SKAMCf@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] c++: constexpr error with fn redecl in local scope [PR111132] | expand |
On 4/4/24 19:15, Marek Polacek wrote: > On Thu, Apr 04, 2024 at 05:28:22PM -0400, Jason Merrill wrote: >> On 4/4/24 14:43, Marek Polacek wrote: >>> On Wed, Apr 03, 2024 at 01:14:46PM -0400, Jason Merrill wrote: >>>> On 4/2/24 13:52, Marek Polacek wrote: >>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/13? >>>>> >>>>> -- >8 -- >>>>> We evaluate constexpr functions on the original, pre-genericization bodies. >>>>> That means that the function body we're evaluating will not have gone >>>>> through cp_genericize_r's "Map block scope extern declarations to visible >>>>> declarations with the same name and type in outer scopes if any". Here: >>>>> >>>>> constexpr bool bar() { return true; } // #1 >>>>> constexpr bool foo() { >>>>> constexpr bool bar(void); // #2 >>>>> return bar(); >>>>> } >>>>> >>>>> it means that we: >>>>> 1) register_constexpr_fundef (#1) >>>>> 2) cp_genericize (#1) >>>>> nothing interesting happens >>>>> 3) register_constexpr_fundef (foo) >>>>> does copy_fn, so we have two copies of the BIND_EXPR >>>>> 4) cp_genericize (foo) >>>>> this remaps #2 to #1, but only on one copy of the BIND_EXPR >>>>> 5) retrieve_constexpr_fundef (foo) >>>>> we find it, no problem >>>>> 6) retrieve_constexpr_fundef (#2) >>>>> and here #2 isn't found in constexpr_fundef_table, because >>>>> we're working on the BIND_EXPR copy where #2 wasn't mapped to #1 >>>>> so we fail. We've only registered #1. >>>>> >>>>> It should work to use DECL_LOCAL_DECL_ALIAS (which used to be >>>>> extern_decl_map). We evaluate constexpr functions on pre-cp_fold >>>>> bodies to avoid diagnostic problems, but the remapping I'm proposing >>>>> should not interfere with diagnostics. >>>>> >>>>> This is not a problem for a global scope redeclaration; there we go >>>>> through duplicate_decls which keeps the DECL_UID: >>>>> DECL_UID (olddecl) = olddecl_uid; >>>>> and DECL_UID is what constexpr_fundef_hasher::hash uses. >>>>> >>>>> PR c++/111132 >>>>> >>>>> gcc/cp/ChangeLog: >>>>> >>>>> * constexpr.cc (get_function_named_in_call): If there's >>>>> a DECL_LOCAL_DECL_ALIAS, use it. >>>> >>>> Perhaps this function should use cp_get_fndecl_from_callee, and this change >>>> should be made there instead? >>> >>> It doesn't seem that get_function_named_in_call can use cp_get_fndecl_from_callee, >>> (or be replaced with cp_get_callee_fndecl_nofold). We can get e.g. a CALL_EXPR >>> whose CALL_EXPR_FN is a TEMPLATE_ID_EXPR, and get_function_named_in_call >>> returns the TEMPLATE_ID_EXPR whereas cp_get_fndecl_from_callee would return >>> null: >>> >>> if (TREE_CODE (fn) == FUNCTION_DECL) >>> return fn; >>> tree type = TREE_TYPE (fn); >>> if (type == NULL_TREE || !INDIRECT_TYPE_P (type)) >>> return NULL_TREE; >> >> Why couldn't this function use cp_get_fndecl_from_callee and return the >> original argument if that function returns null? > > It could. (The lambda below could have been a goto but either should be fine.) > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/13? OK. > -- >8 -- > We evaluate constexpr functions on the original, pre-genericization bodies. > That means that the function body we're evaluating will not have gone > through cp_genericize_r's "Map block scope extern declarations to visible > declarations with the same name and type in outer scopes if any". Here: > > constexpr bool bar() { return true; } // #1 > constexpr bool foo() { > constexpr bool bar(void); // #2 > return bar(); > } > > it means that we: > 1) register_constexpr_fundef (#1) > 2) cp_genericize (#1) > nothing interesting happens > 3) register_constexpr_fundef (foo) > does copy_fn, so we have two copies of the BIND_EXPR > 4) cp_genericize (foo) > this remaps #2 to #1, but only on one copy of the BIND_EXPR > 5) retrieve_constexpr_fundef (foo) > we find it, no problem > 6) retrieve_constexpr_fundef (#2) > and here #2 isn't found in constexpr_fundef_table, because > we're working on the BIND_EXPR copy where #2 wasn't mapped to #1 > so we fail. We've only registered #1. > > It should work to use DECL_LOCAL_DECL_ALIAS (which used to be > extern_decl_map). We evaluate constexpr functions on pre-cp_fold > bodies to avoid diagnostic problems, but the remapping I'm proposing > should not interfere with diagnostics. > > This is not a problem for a global scope redeclaration; there we go > through duplicate_decls which keeps the DECL_UID: > DECL_UID (olddecl) = olddecl_uid; > and DECL_UID is what constexpr_fundef_hasher::hash uses. > > PR c++/111132 > > gcc/cp/ChangeLog: > > * constexpr.cc (get_function_named_in_call): Use > cp_get_fndecl_from_callee. > * cvt.cc (cp_get_fndecl_from_callee): If there's a > DECL_LOCAL_DECL_ALIAS, use it. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/constexpr-redeclaration3.C: New test. > * g++.dg/cpp0x/constexpr-redeclaration4.C: New test. > --- > gcc/cp/constexpr.cc | 10 ++++------ > gcc/cp/cvt.cc | 18 ++++++++++++++++-- > .../g++.dg/cpp0x/constexpr-redeclaration3.C | 13 +++++++++++++ > .../g++.dg/cpp0x/constexpr-redeclaration4.C | 14 ++++++++++++++ > 4 files changed, 47 insertions(+), 8 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-redeclaration3.C > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-redeclaration4.C > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > index fa346fe01c9..410ccdf597f 100644 > --- a/gcc/cp/constexpr.cc > +++ b/gcc/cp/constexpr.cc > @@ -702,16 +702,14 @@ build_constexpr_constructor_member_initializers (tree type, tree body) > > /* We have an expression tree T that represents a call, either CALL_EXPR > or AGGR_INIT_EXPR. If the call is lexically to a named function, > - retrun the _DECL for that function. */ > + return the _DECL for that function. */ > > static tree > get_function_named_in_call (tree t) > { > - tree fun = cp_get_callee (t); > - if (fun && TREE_CODE (fun) == ADDR_EXPR > - && TREE_CODE (TREE_OPERAND (fun, 0)) == FUNCTION_DECL) > - fun = TREE_OPERAND (fun, 0); > - return fun; > + tree callee = cp_get_callee (t); > + tree fun = cp_get_fndecl_from_callee (callee, /*fold*/false); > + return fun ? fun : callee; > } > > /* Subroutine of check_constexpr_fundef. BODY is the body of a function > diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc > index cbed847b343..db086c017e8 100644 > --- a/gcc/cp/cvt.cc > +++ b/gcc/cp/cvt.cc > @@ -1001,8 +1001,22 @@ cp_get_fndecl_from_callee (tree fn, bool fold /* = true */) > { > if (fn == NULL_TREE) > return fn; > + > + /* We evaluate constexpr functions on the original, pre-genericization > + bodies. So block-scope extern declarations have not been mapped to > + declarations in outer scopes. Use the namespace-scope declaration, > + if any, so that retrieve_constexpr_fundef can find it (PR111132). */ > + auto fn_or_local_alias = [] (tree f) > + { > + if (DECL_LOCAL_DECL_P (f)) > + if (tree alias = DECL_LOCAL_DECL_ALIAS (f)) > + if (alias != error_mark_node) > + return alias; > + return f; > + }; > + > if (TREE_CODE (fn) == FUNCTION_DECL) > - return fn; > + return fn_or_local_alias (fn); > tree type = TREE_TYPE (fn); > if (type == NULL_TREE || !INDIRECT_TYPE_P (type)) > return NULL_TREE; > @@ -1013,7 +1027,7 @@ cp_get_fndecl_from_callee (tree fn, bool fold /* = true */) > || TREE_CODE (fn) == FDESC_EXPR) > fn = TREE_OPERAND (fn, 0); > if (TREE_CODE (fn) == FUNCTION_DECL) > - return fn; > + return fn_or_local_alias (fn); > return NULL_TREE; > } > > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-redeclaration3.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-redeclaration3.C > new file mode 100644 > index 00000000000..2b41b456fc3 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-redeclaration3.C > @@ -0,0 +1,13 @@ > +// PR c++/111132 > +// { dg-do compile { target c++11 } } > + > +constexpr bool bar(void) { > + return true; > +} > + > +constexpr bool foo() { > + constexpr bool bar(void); > + return bar(); > +} > + > +static_assert(foo(), ""); > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-redeclaration4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-redeclaration4.C > new file mode 100644 > index 00000000000..c58247218c6 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-redeclaration4.C > @@ -0,0 +1,14 @@ > +// PR c++/111132 > +// { dg-do compile { target c++11 } } > + > +constexpr bool bar(void) { > + return true; > +} > + > +constexpr bool bar(void); > + > +constexpr bool foo() { > + return bar(); > +} > + > +static_assert(foo(), ""); > > base-commit: 27b6d081f68528435066be2234c7329e31e0e84f
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index fa346fe01c9..410ccdf597f 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -702,16 +702,14 @@ build_constexpr_constructor_member_initializers (tree type, tree body) /* We have an expression tree T that represents a call, either CALL_EXPR or AGGR_INIT_EXPR. If the call is lexically to a named function, - retrun the _DECL for that function. */ + return the _DECL for that function. */ static tree get_function_named_in_call (tree t) { - tree fun = cp_get_callee (t); - if (fun && TREE_CODE (fun) == ADDR_EXPR - && TREE_CODE (TREE_OPERAND (fun, 0)) == FUNCTION_DECL) - fun = TREE_OPERAND (fun, 0); - return fun; + tree callee = cp_get_callee (t); + tree fun = cp_get_fndecl_from_callee (callee, /*fold*/false); + return fun ? fun : callee; } /* Subroutine of check_constexpr_fundef. BODY is the body of a function diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc index cbed847b343..db086c017e8 100644 --- a/gcc/cp/cvt.cc +++ b/gcc/cp/cvt.cc @@ -1001,8 +1001,22 @@ cp_get_fndecl_from_callee (tree fn, bool fold /* = true */) { if (fn == NULL_TREE) return fn; + + /* We evaluate constexpr functions on the original, pre-genericization + bodies. So block-scope extern declarations have not been mapped to + declarations in outer scopes. Use the namespace-scope declaration, + if any, so that retrieve_constexpr_fundef can find it (PR111132). */ + auto fn_or_local_alias = [] (tree f) + { + if (DECL_LOCAL_DECL_P (f)) + if (tree alias = DECL_LOCAL_DECL_ALIAS (f)) + if (alias != error_mark_node) + return alias; + return f; + }; + if (TREE_CODE (fn) == FUNCTION_DECL) - return fn; + return fn_or_local_alias (fn); tree type = TREE_TYPE (fn); if (type == NULL_TREE || !INDIRECT_TYPE_P (type)) return NULL_TREE; @@ -1013,7 +1027,7 @@ cp_get_fndecl_from_callee (tree fn, bool fold /* = true */) || TREE_CODE (fn) == FDESC_EXPR) fn = TREE_OPERAND (fn, 0); if (TREE_CODE (fn) == FUNCTION_DECL) - return fn; + return fn_or_local_alias (fn); return NULL_TREE; } diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-redeclaration3.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-redeclaration3.C new file mode 100644 index 00000000000..2b41b456fc3 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-redeclaration3.C @@ -0,0 +1,13 @@ +// PR c++/111132 +// { dg-do compile { target c++11 } } + +constexpr bool bar(void) { + return true; +} + +constexpr bool foo() { + constexpr bool bar(void); + return bar(); +} + +static_assert(foo(), ""); diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-redeclaration4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-redeclaration4.C new file mode 100644 index 00000000000..c58247218c6 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-redeclaration4.C @@ -0,0 +1,14 @@ +// PR c++/111132 +// { dg-do compile { target c++11 } } + +constexpr bool bar(void) { + return true; +} + +constexpr bool bar(void); + +constexpr bool foo() { + return bar(); +} + +static_assert(foo(), "");