[PATCH] c++: Implement LWG3396 Clarify point of reference for source_location::current() [PR80780, PR93093]
Jason Merrill
jason@redhat.com
Tue Dec 1 21:05:22 GMT 2020
On 12/1/20 3:59 AM, Jakub Jelinek wrote:
> Hi!
>
> While std::source_location::current () is static consteval source_location
> current() noexcept; in the standard, it also says with LWG3396:
> "Any call to current that appears as a default member initializer
> ([class.mem]), or as a subexpression thereof, should correspond to the
> location of the constructor definition or aggregate initialization that uses
> the default member initializer. Any call to current that appears as a
> default argument ([dcl.fct.default]), or as a subexpression thereof, should
> correspond to the location of the invocation of the function that uses the
> default argument ([expr.call])."
> so it must work as compiler magic rather than normal immediate functions,
> in particular we need to defer its evaluation when parsing default arguments
> or nsdmis.
....
> The problem is that with your/JeanHeyd's testcase above, the default
> argument parsing is deferred until the whole class is parsed and then
> cp_parser_late_parsing_default_args performs that deferred parsing.
> Except at this point nothing creates the sk_function_params scope.
>
> So, I think we need some way for the build_over_call code to know if
> it is called from within cp_parser_late_parsing_default_args and whether
> for an immediate function or not, and take that into account too.
>
> I see cp_parser_late_parsing_default_args calls push_defarg_context,
> so perhaps it could check default_arg_context vector, except that
> convert_default_arg calls that too around calling break_out_target_exprs
> which in the patch is used to perform the late evaluation of the
> std::source_location::current() calls. So it might need to temporarily
> pop_defarg_context before break_out_target_exprs and push it again?
How about forcing the constant evaluation in bot_manip?
Or simpler might be to always defer immediate evaluation of
source_location::current() until genericize time.
> And guess we need some tests if it works as expected also in templates;
Please.
> 2020-12-01 Jakub Jelinek <jakub@redhat.com>
>
> LWG3396 Clarify point of reference for source_location::current()
> PR c++/80780
> PR c++/93093
> * cp-tree.h (source_location_current_p): Declare.
> * tree.c (source_location_current_p): New function.
> (bot_manip): Resolve deferred calls to immediate function
> std::source_location::current ().
> * call.c (build_over_call): Don't evaluate calls to immediate
> function std::source_location::current () in default arguments
> or when parsing nsdmi.
>
> * g++.dg/cpp2a/srcloc15.C: New test.
> * g++.dg/cpp2a/srcloc16.C: New test.
>
> --- gcc/cp/cp-tree.h.jj 2020-11-26 16:22:24.252407018 +0100
> +++ gcc/cp/cp-tree.h 2020-11-30 19:45:36.784286305 +0100
> @@ -7413,6 +7413,7 @@ extern tree bind_template_template_parm
> extern tree array_type_nelts_total (tree);
> extern tree array_type_nelts_top (tree);
> extern bool array_of_unknown_bound_p (const_tree);
> +extern bool source_location_current_p (tree);
> extern tree break_out_target_exprs (tree, bool = false);
> extern tree build_ctor_subob_ref (tree, tree, tree);
> extern tree replace_placeholders (tree, tree, bool * = NULL);
> --- gcc/cp/tree.c.jj 2020-11-26 16:22:24.264406884 +0100
> +++ gcc/cp/tree.c 2020-11-30 19:53:58.034686074 +0100
> @@ -2968,6 +2968,37 @@ array_type_nelts_total (tree type)
> return sz;
> }
>
> +/* Return true if FNDECL is std::source_location::current () method. */
> +
> +bool
> +source_location_current_p (tree fndecl)
> +{
> + gcc_checking_assert (TREE_CODE (fndecl) == FUNCTION_DECL
> + && DECL_IMMEDIATE_FUNCTION_P (fndecl));
> + if (DECL_NAME (fndecl) == NULL_TREE
> + || TREE_CODE (TREE_TYPE (fndecl)) != FUNCTION_TYPE
> + || TREE_CODE (TREE_TYPE (TREE_TYPE (fndecl))) != RECORD_TYPE
> + || DECL_CONTEXT (fndecl) != TREE_TYPE (TREE_TYPE (fndecl)))
> + return false;
> +
> + if (strcmp (IDENTIFIER_POINTER (DECL_NAME (fndecl)), "current") != 0)
You can use id_equal for comparing identifiers to strings.
> + return false;
> +
> + tree source_location = DECL_CONTEXT (fndecl);
> + if (TYPE_NAME (source_location) == NULL_TREE
> + || TREE_CODE (TYPE_NAME (source_location)) != TYPE_DECL
> + || DECL_NAME (TYPE_NAME (source_location)) == NULL_TREE
> + || strcmp (IDENTIFIER_POINTER (DECL_NAME (TYPE_NAME (source_location))),
> + "source_location") != 0)
TYPE_IDENTIFIER would make this shorter.
> + return false;
> +
> + tree decl
> + = lookup_qualified_name (std_node,
> + DECL_NAME (TYPE_NAME (source_location)),
> + LOOK_want::TYPE, tf_none);
> + return TYPE_NAME (source_location) == decl;
Why not TYPE_CONTEXT (source_location) == std_node? I don't think we
need to do name lookup here.
> +}
> +
> struct bot_data
> {
> splay_tree target_remap;
> @@ -3064,6 +3095,38 @@ bot_manip (tree* tp, int* walk_subtrees,
> set_flags_from_callee (*tp);
> if (data.clear_location && EXPR_HAS_LOCATION (*tp))
> SET_EXPR_LOCATION (*tp, input_location);
> + if (TREE_CODE (*tp) == CALL_EXPR
> + && !processing_template_decl
> + && (current_function_decl == NULL_TREE
> + || !DECL_IMMEDIATE_FUNCTION_P (current_function_decl)))
> + if (tree fndecl = cp_get_callee_fndecl_nofold (*tp))
> + if (DECL_IMMEDIATE_FUNCTION_P (fndecl)
> + && source_location_current_p (fndecl))
> + {
> + bool ok = call_expr_nargs (*tp) == 0;
> + if (call_expr_nargs (*tp) == 1
> + && TREE_CODE (CALL_EXPR_ARG (*tp, 0)) == CALL_EXPR)
> + if (tree fndecl2
> + = cp_get_callee_fndecl_nofold (CALL_EXPR_ARG (*tp, 0)))
> + if (fndecl_built_in_p (fndecl2, CP_BUILT_IN_SOURCE_LOCATION,
> + BUILT_IN_FRONTEND))
> + ok = true;
> + if (!ok)
> + {
> + error_at (EXPR_LOCATION (*tp),
> + "%qD called with arguments", fndecl);
> + *tp = build_constructor (TREE_TYPE (TREE_TYPE (fndecl)),
> + NULL);
> + }
> + else
> + {
> + vec<tree, va_gc> *args = NULL;
> + push_deferring_access_checks (dk_no_check);
> + *tp = build_new_function_call (fndecl, &args,
> + tf_warning_or_error);
> + pop_deferring_access_checks ();
> + }
> + }
> return t;
> }
>
> --- gcc/cp/call.c.jj 2020-11-18 09:40:09.000000000 +0100
> +++ gcc/cp/call.c 2020-11-30 19:47:03.951312431 +0100
> @@ -8613,7 +8613,9 @@ build_over_call (struct z_candidate *can
> && (current_function_decl == NULL_TREE
> || !DECL_IMMEDIATE_FUNCTION_P (current_function_decl))
> && (current_binding_level->kind != sk_function_parms
> - || !current_binding_level->immediate_fn_ctx_p))
> + || (!current_binding_level->immediate_fn_ctx_p
> + && !source_location_current_p (fn)))
> + && (!parsing_nsdmi () || !source_location_current_p (fn)))
> {
> tree obj_arg = NULL_TREE, exprimm = expr;
> if (DECL_CONSTRUCTOR_P (fn))
Please factor this condition out into a separate function.
> @@ -9257,7 +9259,9 @@ build_over_call (struct z_candidate *can
> && (current_function_decl == NULL_TREE
> || !DECL_IMMEDIATE_FUNCTION_P (current_function_decl))
> && (current_binding_level->kind != sk_function_parms
> - || !current_binding_level->immediate_fn_ctx_p))
> + || (!current_binding_level->immediate_fn_ctx_p
> + && !source_location_current_p (fndecl)))
> + && (!parsing_nsdmi () || !source_location_current_p (fndecl)))
> {
> tree obj_arg = NULL_TREE;
> if (DECL_CONSTRUCTOR_P (fndecl))
> --- gcc/testsuite/g++.dg/cpp2a/srcloc15.C.jj 2020-11-30 17:57:41.506279816 +0100
> +++ gcc/testsuite/g++.dg/cpp2a/srcloc15.C 2020-11-30 17:57:21.169507324 +0100
> @@ -0,0 +1,82 @@
> +// { dg-do compile { target c++20 } }
> +
> +namespace std {
> + struct source_location {
> + struct __impl {
> + const char *_M_file_name;
> + const char *_M_function_name;
> + unsigned int _M_line, _M_column;
> + };
> + const __impl *__ptr;
> + constexpr source_location () : __ptr (nullptr) {}
> + static consteval source_location
> + current (const void *__p = __builtin_source_location ()) {
> + source_location __ret;
> + __ret.__ptr = static_cast <const __impl *> (__p);
> + return __ret;
> + }
> + constexpr const char *file_name () const {
> + return __ptr ? __ptr->_M_file_name : "";
> + }
> + constexpr const char *function_name () const {
> + return __ptr ? __ptr->_M_function_name : "";
> + }
> + constexpr unsigned line () const {
> + return __ptr ? __ptr->_M_line : 0;
> + }
> + constexpr unsigned column () const {
> + return __ptr ? __ptr->_M_column : 0;
> + }
> + };
> +}
> +
> +using namespace std;
> +
> +constexpr source_location
> +foo (const source_location x = source_location::current ())
> +{
> + return x;
> +}
> +
> +constexpr bool
> +bar ()
> +{
> + int line = __LINE__;
> + source_location a = foo ();
> + source_location b = source_location::current ();
> + source_location c = foo ();
> + // ^ column 28
> + // ^ column 49
> + const source_location *d[3] = { &a, &b, &c };
> + const char *file1 = __FILE__;
> + const char *function1 = __FUNCTION__;
> + for (int j = 0; j < 3; j++)
> + {
> + int i= 0;
> + const char *file2 = d[j]->file_name ();
> + const char *function2 = d[j]->function_name ();
> + for (i = 0; file1[i]; i++)
> + if (file1[i] != file2[i])
> + return false;
> + if (file2[i])
> + return false;
> + for (i = 0; function1[i]; i++)
> + if (function1[i] != function2[i])
> + return false;
> + if (function2[i])
> + return false;
> + if (d[j]->line () != line + j + 1)
> + return false;
> + if (d[j]->column () != (j == 1 ? 49 : 28))
> + return false;
> + }
> + return true;
> +}
> +
> +static_assert (bar ());
> +
> +int
> +main ()
> +{
> + bar ();
> +}
> --- gcc/testsuite/g++.dg/cpp2a/srcloc16.C.jj 2020-11-30 20:15:25.416283572 +0100
> +++ gcc/testsuite/g++.dg/cpp2a/srcloc16.C 2020-11-30 20:15:06.285497601 +0100
> @@ -0,0 +1,96 @@
> +// { dg-do compile { target c++20 } }
> +
> +namespace std {
> + struct source_location {
> + struct __impl {
> + const char *_M_file_name;
> + const char *_M_function_name;
> + unsigned int _M_line, _M_column;
> + };
> + const __impl *__ptr;
> + constexpr source_location () : __ptr (nullptr) {}
> + static consteval source_location
> + current (const void *__p = __builtin_source_location ()) {
> + source_location __ret;
> + __ret.__ptr = static_cast <const __impl *> (__p);
> + return __ret;
> + }
> + constexpr const char *file_name () const {
> + return __ptr ? __ptr->_M_file_name : "";
> + }
> + constexpr const char *function_name () const {
> + return __ptr ? __ptr->_M_function_name : "";
> + }
> + constexpr unsigned line () const {
> + return __ptr ? __ptr->_M_line : 0;
> + }
> + constexpr unsigned column () const {
> + return __ptr ? __ptr->_M_column : 0;
> + }
> + };
> +}
> +
> +using namespace std;
> +
> +struct S
> +{
> + source_location a = source_location::current ();
> + source_location b = source_location::current ();
> + source_location c = source_location ();
> + constexpr S () { c = source_location::current (); }
> +};
> +
> +struct T
> +{
> + int t;
> + source_location u = source_location::current ();
> + int v = __builtin_LINE ();
> +};
> +
> +constexpr S s;
> +constexpr T t = { 1 };
> +
> +constexpr bool
> +cmp (const char *p, const char *q)
> +{
> + for (; *p && *q; p++, q++)
> + if (*p != *q)
> + return true;
> + return *p || *q;
> +}
> +
> +constexpr bool
> +foo ()
> +{
> + T u = { 2 };
> + source_location v = source_location::current ();
> + if (cmp (s.a.file_name (), s.c.file_name ())
> + || cmp (s.b.file_name (), s.c.file_name ())
> + || cmp (t.u.file_name (), s.c.file_name ())
> + || cmp (u.u.file_name (), s.c.file_name ())
> + || cmp (v.file_name (), s.c.file_name ())
> + || cmp (s.a.function_name (), s.c.function_name ())
> + || cmp (s.b.function_name (), s.c.function_name ())
> + || cmp (t.u.function_name (), "")
> + || cmp (u.u.function_name (), v.function_name ())
> + || s.a.line () != s.c.line ()
> + || s.b.line () != s.c.line ()
> + || t.u.line () != t.v
> + || u.u.line () + 1 != v.line ()
> + || s.a.column () != 18
> + || s.b.column () != 18
> + || s.c.column () != 50
> + || t.u.column () != 21
> + || u.u.column () != 13
> + || v.column () != 49)
> + return false;
> + return true;
> +}
> +
> +static_assert (foo ());
> +
> +int
> +main ()
> +{
> + foo ();
> +}
>
> Jakub
>
More information about the Gcc-patches
mailing list