Message ID | 20240402175237.482119-1-polacek@redhat.com |
---|---|
State | New |
Headers | show |
Series | c++: constexpr error with fn redecl in local scope [PR111132] | expand |
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? Jason
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; Marek
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? Jason
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index fa346fe01c9..b47f0e984c0 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -702,15 +702,26 @@ 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); + if (fun) + { + if (TREE_CODE (fun) == ADDR_EXPR + && TREE_CODE (TREE_OPERAND (fun, 0)) == FUNCTION_DECL) + fun = TREE_OPERAND (fun, 0); + /* 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). */ + if (TREE_CODE (fun) == FUNCTION_DECL && DECL_LOCAL_DECL_P (fun)) + if (tree alias = DECL_LOCAL_DECL_ALIAS (fun)) + if (alias != error_mark_node) + fun = alias; + } return fun; } 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(), "");