Re: [C++ PATCH] __builtin_source_location ()
On Tue, Dec 03, 2019 at 03:37:41PM -0500, Jason Merrill wrote:
> On 11/15/19 7:28 AM, Jakub Jelinek wrote:
> > + loc = LOCATION_LOCUS (loc);
> ...
> > + entry.loc
> > += linemap_resolve_location (line_table, loc, LRK_MACRO_EXPANSION_POINT,
> > + );
>
> You don't need LOCATION_LOCUS if you're calling linemap_resolve_location.
> LGTM with that change and David's.
I had to apply also Jon's suggestion to use _M_* names of the members
instead of __* names (didn't use the variant that would support both), while
__impl remains with two underscores. Below is what I'll be retesting.
> > I know David and Jonathan had comments on the patch and can try to
> > incorporate them, but more importantly I'd like to ask whether we want to
> > add this at all or wait till GCC 11 and add something else.
> > Yet another option would be make it clear that the builtin is a temporary
> > thing until we implement something different and then would be removed,
> > say by reporting error if the builtin is used anywhere but in default
> > argument of std::source_location::current or something similar; I mean if
> > the ABI is sound, but we might have __builtin_constexpr_force_static or
> > whatever else or some new expression form one day and replace the builtin
> > uses with that.
>
> I don't think we need to worry about enforcement; we say in general that
> C++20 support is experimental, and we don't try to maintain compatibility;
> even more so for internal details like this.
2019-12-03 Jakub Jelinek
* cp-tree.h (enum cp_tree_index): Add CPTI_SOURCE_LOCATION_IMPL.
(source_location_impl): Define.
(enum cp_built_in_function): Add CP_BUILT_IN_SOURCE_LOCATION.
(fold_builtin_source_location): Declare.
* cp-gimplify.c: Include output.h, file-prefix-map.h and cgraph.h.
(cp_gimplify_expr, cp_fold): Handle CP_BUILT_IN_SOURCE_LOCATION.
Formatting fix.
(get_source_location_impl_type): New function.
(struct source_location_table_entry,
struct source_location_table_entry_hash): New types.
(source_location_table, source_location_id): New variables.
(fold_builtin_source_location): New function.
* constexpr.c (cxx_eval_builtin_function_call): Handle
CP_BUILT_IN_SOURCE_LOCATION.
* tree.c (builtin_valid_in_constant_expr_p): Likewise. Formatting
fix.
* decl.c (cxx_init_decl_processing): Register
__builtin_source_location.
* name-lookup.c (get_std_name_hint): Add source_location entry.
* g++.dg/cpp2a/srcloc1.C: New test.
* g++.dg/cpp2a/srcloc2.C: New test.
* g++.dg/cpp2a/srcloc3.C: New test.
* g++.dg/cpp2a/srcloc4.C: New test.
* g++.dg/cpp2a/srcloc5.C: New test.
* g++.dg/cpp2a/srcloc6.C: New test.
* g++.dg/cpp2a/srcloc7.C: New test.
* g++.dg/cpp2a/srcloc8.C: New test.
* g++.dg/cpp2a/srcloc9.C: New test.
* g++.dg/cpp2a/srcloc10.C: New test.
* g++.dg/cpp2a/srcloc11.C: New test.
* g++.dg/cpp2a/srcloc12.C: New test.
* g++.dg/cpp2a/srcloc13.C: New test.
* g++.dg/cpp2a/srcloc14.C: New test.
--- gcc/cp/cp-tree.h.jj 2019-12-03 20:21:29.743476880 +0100
+++ gcc/cp/cp-tree.h2019-12-03 21:45:31.807989738 +0100
@@ -204,6 +204,8 @@ enum cp_tree_index
CPTI_ANY_TARG,
+CPTI_SOURCE_LOCATION_IMPL,
+
CPTI_MAX
};
@@ -356,6 +358,9 @@ extern GTY(()) tree cp_global_trees[CPTI
/* A node which matches any template argument. */
#define any_targ_node cp_global_trees[CPTI_ANY_TARG]
+/* std::source_location::__impl class. */
+#define source_location_impl
cp_global_trees[CPTI_SOURCE_LOCATION_IMPL]
+
/* Node to indicate default access. This must be distinct from the
access nodes in tree.h. */
@@ -6182,6 +6187,7 @@ struct GTY((chain_next ("%h.next"))) tin
enum cp_built_in_function {
CP_BUILT_IN_IS_CONSTANT_EVALUATED,
CP_BUILT_IN_INTEGER_PACK,
+ CP_BUILT_IN_SOURCE_LOCATION,
CP_BUILT_IN_LAST
};
@@ -7735,6 +7741,7 @@ extern void clear_fold_cache (void);
extern tree lookup_hotness_attribute (tree);
extern tree process_stmt_hotness_attribute (tree, location_t);
extern bool simple_empty_class_p (tree, tree, tree_code);
+extern tree fold_builtin_source_location (location_t);
/* in name-lookup.c */
extern tree strip_using_decl(tree);
--- gcc/cp/cp-gimplify.c.jj 2019-11-27 17:26:57.233013947 +0100
+++ gcc/cp/cp-gimplify.c2019-12-03 21:52:31.720546585 +0100
@@ -35,6 +35,9 @@ along with GCC; see the file COPYING3.
#include "attribs.h"
#include "asan.h"
#include "gcc-rich-location.h"
+#include "output.h"
+#include "file-prefix-map.h"
+#include "cgraph.h"
/* Forward declarations. */
@@ -896,8 +899,12 @@ cp_gimplify_expr (tree *expr_p, gimple_s
tree decl = cp_get_callee_fndecl_nofold (*expr_p);
Re: [C++ PATCH] __builtin_source_location ()
On 11/15/19 7:28 AM, Jakub Jelinek wrote: + loc = LOCATION_LOCUS (loc); ... + entry.loc += linemap_resolve_location (line_table, loc, LRK_MACRO_EXPANSION_POINT, + ); You don't need LOCATION_LOCUS if you're calling linemap_resolve_location. LGTM with that change and David's. I know David and Jonathan had comments on the patch and can try to incorporate them, but more importantly I'd like to ask whether we want to add this at all or wait till GCC 11 and add something else. Yet another option would be make it clear that the builtin is a temporary thing until we implement something different and then would be removed, say by reporting error if the builtin is used anywhere but in default argument of std::source_location::current or something similar; I mean if the ABI is sound, but we might have __builtin_constexpr_force_static or whatever else or some new expression form one day and replace the builtin uses with that. I don't think we need to worry about enforcement; we say in general that C++20 support is experimental, and we don't try to maintain compatibility; even more so for internal details like this. Jason
Patch ping (was Re: [C++ PATCH] __builtin_source_location ())
Hi! On Fri, Nov 15, 2019 at 01:28:17PM +0100, Jakub Jelinek wrote: > On Thu, Nov 14, 2019 at 08:34:26PM +0100, Jakub Jelinek wrote: > > The following WIP patch implements __builtin_source_location (), > > which returns const void pointer to a std::source_location::__impl > > struct that is required to contain __file, __function, __line and __column > > fields, the first two with const char * type, the latter some integral type. > > Here is hopefully final version, with the hashing implemented, > __file renamed to __file_name and __function to __function_name to match > how the standard names the methods and with testsuite coverage. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? I'd like to ping this patch. I know David and Jonathan had comments on the patch and can try to incorporate them, but more importantly I'd like to ask whether we want to add this at all or wait till GCC 11 and add something else. Yet another option would be make it clear that the builtin is a temporary thing until we implement something different and then would be removed, say by reporting error if the builtin is used anywhere but in default argument of std::source_location::current or something similar; I mean if the ABI is sound, but we might have __builtin_constexpr_force_static or whatever else or some new expression form one day and replace the builtin uses with that. > 2019-11-15 Jakub Jelinek > > * cp-tree.h (enum cp_tree_index): Add CPTI_SOURCE_LOCATION_IMPL. > (source_location_impl): Define. > (enum cp_built_in_function): Add CP_BUILT_IN_SOURCE_LOCATION. > (fold_builtin_source_location): Declare. > * cp-gimplify.c: Include output.h, file-prefix-map.h and cgraph.h. > (cp_gimplify_expr, cp_fold): Handle CP_BUILT_IN_SOURCE_LOCATION. > Formatting fix. > (get_source_location_impl): New function. > (struct source_location_table_entry, > struct source_location_table_entry_hash): New types. > (source_location_table, source_location_id): New variables. > (fold_builtin_source_location): New function. > * constexpr.c (cxx_eval_builtin_function_call): Handle > CP_BUILT_IN_SOURCE_LOCATION. > * tree.c (builtin_valid_in_constant_expr_p): Likewise. Formatting > fix. > * decl.c (cxx_init_decl_processing): Register > __builtin_source_location. > * name-lookup.c (get_std_name_hint): Add source_location entry. > > * g++.dg/cpp2a/srcloc1.C: New test. > * g++.dg/cpp2a/srcloc2.C: New test. > * g++.dg/cpp2a/srcloc3.C: New test. > * g++.dg/cpp2a/srcloc4.C: New test. > * g++.dg/cpp2a/srcloc5.C: New test. > * g++.dg/cpp2a/srcloc6.C: New test. > * g++.dg/cpp2a/srcloc7.C: New test. > * g++.dg/cpp2a/srcloc8.C: New test. > * g++.dg/cpp2a/srcloc9.C: New test. > * g++.dg/cpp2a/srcloc10.C: New test. > * g++.dg/cpp2a/srcloc11.C: New test. > * g++.dg/cpp2a/srcloc12.C: New test. > * g++.dg/cpp2a/srcloc13.C: New test. > * g++.dg/cpp2a/srcloc14.C: New test. Jakub
Re: [C++ PATCH] __builtin_source_location ()
On Fri, Nov 15, 2019 at 09:04:17AM -0500, David Malcolm wrote:
> > +/* Helper of fold_builtin_source_location, return the
> > + std::source_location::__impl type after performing verification
> > + on it. */
> > +
> > +static tree
> > +get_source_location_impl (location_t loc)
> > +{
>
> FWIW, I found this function confusing at first, and wondered what the
> purpose of LOC was (given that the function is building a type, rather
> than an instance of that type).
>
> Maybe add this to the leading comment:
> "LOC is used for reporting any errors."
> (and if so, perhaps replace the various "error" with "error_at", though
> that's rather academic).
>
> Maybe rename the function to "get_source_location_impl_type" to
> emphasize the distinction?
Consider it done.
Jakub
Re: [C++ PATCH] __builtin_source_location ()
On 15/11/19 13:49 +0100, Jakub Jelinek wrote: On Fri, Nov 15, 2019 at 12:35:22PM +, Jonathan Wakely wrote: On 15/11/19 13:28 +0100, Jakub Jelinek wrote: > On Thu, Nov 14, 2019 at 08:34:26PM +0100, Jakub Jelinek wrote: > > The following WIP patch implements __builtin_source_location (), > > which returns const void pointer to a std::source_location::__impl > > struct that is required to contain __file, __function, __line and __column > > fields, the first two with const char * type, the latter some integral type. > > Here is hopefully final version, with the hashing implemented, > __file renamed to __file_name and __function to __function_name to match > how the standard names the methods and with testsuite coverage. The libstdc++ naming convention says the data members should be _M_file_name, _M_line etc. Since this is just a normal library type now, not defined by the compiler, I think we should follow the library convention (sorry for not noticing that sooner). I guess it depends on what are the chances other compilers will implement the same builtin, because doesn't say libc++ use __name rather than _M_name convention? Yes, that's what libc++ uses, but I get the impression Clang isn't going to add this built-in anyway. Yet another option would be for the builtin to accept either _M_file_name or __file_name, could be handled by const char *n = IDENTIFIER_POINTER (DECL_NAME (field)); if (strncmp (n, "_M_", 3) == 0) n += 3; else if (strncmp (n, "__", 2) == 0) n += 2; else n = ""; and then do the comparisons without __ in the patch. My inclination is YAGNI. At this point, a libc++ implementation that would want to use __builtin_source_location when compiled with GCC is hypothetical, and would already need various #if conditions around it. BCCing libc++ maintainers in case they want to request that GCC's built-in pre-emptively support a std::source_location::__impl type that uses __ prefixes on its data members. For more context see https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01389.html and the rest of the thread.
Re: [C++ PATCH] __builtin_source_location ()
On Fri, 2019-11-15 at 13:28 +0100, Jakub Jelinek wrote:
> On Thu, Nov 14, 2019 at 08:34:26PM +0100, Jakub Jelinek wrote:
> > The following WIP patch implements __builtin_source_location (),
> > which returns const void pointer to a std::source_location::__impl
> > struct that is required to contain __file, __function, __line and
> > __column
> > fields, the first two with const char * type, the latter some
> > integral type.
>
> Here is hopefully final version, with the hashing implemented,
> __file renamed to __file_name and __function to __function_name to
> match
> how the standard names the methods and with testsuite coverage.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
[...snip...]
> --- gcc/cp/cp-gimplify.c.jj 2019-11-15 00:37:14.511247252 +0100
> +++ gcc/cp/cp-gimplify.c 2019-11-15 11:23:50.574203292 +0100
[...]
> @@ -2868,4 +2883,246 @@ process_stmt_hotness_attribute (tree std
>return std_attrs;
> }
>
> +/* Helper of fold_builtin_source_location, return the
> + std::source_location::__impl type after performing verification
> + on it. */
> +
> +static tree
> +get_source_location_impl (location_t loc)
> +{
FWIW, I found this function confusing at first, and wondered what the
purpose of LOC was (given that the function is building a type, rather
than an instance of that type).
Maybe add this to the leading comment:
"LOC is used for reporting any errors."
(and if so, perhaps replace the various "error" with "error_at", though
that's rather academic).
Maybe rename the function to "get_source_location_impl_type" to
emphasize the distinction?
Hope this is constructive
Dave
> + tree name = get_identifier ("source_location");
> + tree decl = lookup_qualified_name (std_node, name);
> + if (TREE_CODE (decl) != TYPE_DECL)
> +{
> + auto_diagnostic_group d;
> + if (decl == error_mark_node || TREE_CODE (decl) == TREE_LIST)
> + qualified_name_lookup_error (std_node, name, decl, loc);
> + else
> + error ("%qD is not a type", decl);
> + return error_mark_node;
> +}
> + name = get_identifier ("__impl");
> + tree type = TREE_TYPE (decl);
> + decl = lookup_qualified_name (type, name);
> + if (TREE_CODE (decl) != TYPE_DECL)
> +{
> + auto_diagnostic_group d;
> + if (decl == error_mark_node || TREE_CODE (decl) == TREE_LIST)
> + qualified_name_lookup_error (type, name, decl, loc);
> + else
> + error ("%qD is not a type", decl);
> + return error_mark_node;
> +}
> + type = TREE_TYPE (decl);
> + if (TREE_CODE (type) != RECORD_TYPE)
> +{
> + error ("%qD is not a class type", decl);
> + return error_mark_node;
> +}
> +
> + int cnt = 0;
> + for (tree field = TYPE_FIELDS (type);
> + (field = next_initializable_field (field)) != NULL_TREE;
> + field = DECL_CHAIN (field))
> +{
> + if (DECL_NAME (field) != NULL_TREE)
> + {
> + const char *n = IDENTIFIER_POINTER (DECL_NAME (field));
> + if (strcmp (n, "__file_name") == 0
> + || strcmp (n, "__function_name") == 0)
> + {
> + if (TREE_TYPE (field) != const_string_type_node)
> + {
> + error ("%qD does not have % type",
> field);
> + return error_mark_node;
> + }
> + cnt++;
> + continue;
> + }
> + else if (strcmp (n, "__line") == 0 || strcmp (n, "__column")
> == 0)
> + {
> + if (TREE_CODE (TREE_TYPE (field)) != INTEGER_TYPE)
> + {
> + error ("%qD does not have integral type", field);
> + return error_mark_node;
> + }
> + cnt++;
> + continue;
> + }
> + }
> + cnt = 0;
> + break;
> +}
> + if (cnt != 4)
> +{
> + error ("% does not contain only
> "
> + "non-static data members %<__file_name%>,
> %<__function_name%>, "
> + "%<__line%> and %<__column%>");
> + return error_mark_node;
> +}
> + return build_qualified_type (type, TYPE_QUAL_CONST);
> +}
[...snip...]
Re: [C++ PATCH] __builtin_source_location ()
On Fri, Nov 15, 2019 at 12:35:22PM +, Jonathan Wakely wrote: > On 15/11/19 13:28 +0100, Jakub Jelinek wrote: > > On Thu, Nov 14, 2019 at 08:34:26PM +0100, Jakub Jelinek wrote: > > > The following WIP patch implements __builtin_source_location (), > > > which returns const void pointer to a std::source_location::__impl > > > struct that is required to contain __file, __function, __line and __column > > > fields, the first two with const char * type, the latter some integral > > > type. > > > > Here is hopefully final version, with the hashing implemented, > > __file renamed to __file_name and __function to __function_name to match > > how the standard names the methods and with testsuite coverage. > > The libstdc++ naming convention says the data members should be > _M_file_name, _M_line etc. > > Since this is just a normal library type now, not defined by the > compiler, I think we should follow the library convention (sorry for > not noticing that sooner). I guess it depends on what are the chances other compilers will implement the same builtin, because doesn't say libc++ use __name rather than _M_name convention? Yet another option would be for the builtin to accept either _M_file_name or __file_name, could be handled by const char *n = IDENTIFIER_POINTER (DECL_NAME (field)); if (strncmp (n, "_M_", 3) == 0) n += 3; else if (strncmp (n, "__", 2) == 0) n += 2; else n = ""; and then do the comparisons without __ in the patch. Jakub
Re: [C++ PATCH] __builtin_source_location ()
On 15/11/19 13:28 +0100, Jakub Jelinek wrote: On Thu, Nov 14, 2019 at 08:34:26PM +0100, Jakub Jelinek wrote: The following WIP patch implements __builtin_source_location (), which returns const void pointer to a std::source_location::__impl struct that is required to contain __file, __function, __line and __column fields, the first two with const char * type, the latter some integral type. Here is hopefully final version, with the hashing implemented, __file renamed to __file_name and __function to __function_name to match how the standard names the methods and with testsuite coverage. The libstdc++ naming convention says the data members should be _M_file_name, _M_line etc. Since this is just a normal library type now, not defined by the compiler, I think we should follow the library convention (sorry for not noticing that sooner).
Re: [RFC C++ PATCH] __builtin_source_location ()
On 14/11/19 23:39 +0100, Jakub Jelinek wrote:
On Thu, Nov 14, 2019 at 10:15:21PM +, Jonathan Wakely wrote:
> namespace std {
> struct source_location {
>struct __impl {
Will this work if the library type is actually in an inline namespace,
such as std::__8::source_location::__impl (as for
--enable-symvers=gnu-versioned-namespace) or
std::__v1::source_location::__impl (as it probably would be in
libc++).
If I'm reading the patch correctly, it would work fine, because
qualified lookup for std::source_location would find that name even if
it's really in some inline namespace.
I'd say so, but the unfinished testsuite coverage would need to cover it of
course.
> const char *__file;
> const char *__function;
> unsigned int __line, __column;
>};
>const void *__ptr;
If the magic type the compiler generates is declared in the header,
then this member might as well be 'const __impl*'.
Yes, with the static_cast on the __builtin_source_location ()
result sure. I can't easily make it return const std::source_location::__impl*
though, because the initialization of the builtins happens early, before
is parsed.
And as it is a nested class, I think I can't predeclare it in the compiler.
If it would be std::__source_location_impl instead of
std::source_location::__impl, perhaps I could pretend there is
namespace std { struct __source_location_impl; }
but I bet that wouldn't work well with the inline namespaces.
So, is const void * return from the builtin ok?
Yes, that seems fine. The library can just do the cast once and store
the result, instead of casting it on every use.
[C++ PATCH] __builtin_source_location ()
On Thu, Nov 14, 2019 at 08:34:26PM +0100, Jakub Jelinek wrote:
> The following WIP patch implements __builtin_source_location (),
> which returns const void pointer to a std::source_location::__impl
> struct that is required to contain __file, __function, __line and __column
> fields, the first two with const char * type, the latter some integral type.
Here is hopefully final version, with the hashing implemented,
__file renamed to __file_name and __function to __function_name to match
how the standard names the methods and with testsuite coverage.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-11-15 Jakub Jelinek
* cp-tree.h (enum cp_tree_index): Add CPTI_SOURCE_LOCATION_IMPL.
(source_location_impl): Define.
(enum cp_built_in_function): Add CP_BUILT_IN_SOURCE_LOCATION.
(fold_builtin_source_location): Declare.
* cp-gimplify.c: Include output.h, file-prefix-map.h and cgraph.h.
(cp_gimplify_expr, cp_fold): Handle CP_BUILT_IN_SOURCE_LOCATION.
Formatting fix.
(get_source_location_impl): New function.
(struct source_location_table_entry,
struct source_location_table_entry_hash): New types.
(source_location_table, source_location_id): New variables.
(fold_builtin_source_location): New function.
* constexpr.c (cxx_eval_builtin_function_call): Handle
CP_BUILT_IN_SOURCE_LOCATION.
* tree.c (builtin_valid_in_constant_expr_p): Likewise. Formatting
fix.
* decl.c (cxx_init_decl_processing): Register
__builtin_source_location.
* name-lookup.c (get_std_name_hint): Add source_location entry.
* g++.dg/cpp2a/srcloc1.C: New test.
* g++.dg/cpp2a/srcloc2.C: New test.
* g++.dg/cpp2a/srcloc3.C: New test.
* g++.dg/cpp2a/srcloc4.C: New test.
* g++.dg/cpp2a/srcloc5.C: New test.
* g++.dg/cpp2a/srcloc6.C: New test.
* g++.dg/cpp2a/srcloc7.C: New test.
* g++.dg/cpp2a/srcloc8.C: New test.
* g++.dg/cpp2a/srcloc9.C: New test.
* g++.dg/cpp2a/srcloc10.C: New test.
* g++.dg/cpp2a/srcloc11.C: New test.
* g++.dg/cpp2a/srcloc12.C: New test.
* g++.dg/cpp2a/srcloc13.C: New test.
* g++.dg/cpp2a/srcloc14.C: New test.
--- gcc/cp/cp-tree.h.jj 2019-11-15 00:37:14.282250695 +0100
+++ gcc/cp/cp-tree.h2019-11-15 09:39:58.514862699 +0100
@@ -204,6 +204,8 @@ enum cp_tree_index
CPTI_ANY_TARG,
+CPTI_SOURCE_LOCATION_IMPL,
+
CPTI_MAX
};
@@ -356,6 +358,9 @@ extern GTY(()) tree cp_global_trees[CPTI
/* A node which matches any template argument. */
#define any_targ_node cp_global_trees[CPTI_ANY_TARG]
+/* std::source_location::__impl class. */
+#define source_location_impl
cp_global_trees[CPTI_SOURCE_LOCATION_IMPL]
+
/* Node to indicate default access. This must be distinct from the
access nodes in tree.h. */
@@ -6175,6 +6180,7 @@ struct GTY((chain_next ("%h.next"))) tin
enum cp_built_in_function {
CP_BUILT_IN_IS_CONSTANT_EVALUATED,
CP_BUILT_IN_INTEGER_PACK,
+ CP_BUILT_IN_SOURCE_LOCATION,
CP_BUILT_IN_LAST
};
@@ -7723,6 +7729,7 @@ extern void clear_fold_cache (void);
extern tree lookup_hotness_attribute (tree);
extern tree process_stmt_hotness_attribute (tree, location_t);
extern bool simple_empty_class_p (tree, tree, tree_code);
+extern tree fold_builtin_source_location (location_t);
/* in name-lookup.c */
extern tree strip_using_decl(tree);
--- gcc/cp/cp-gimplify.c.jj 2019-11-15 00:37:14.511247252 +0100
+++ gcc/cp/cp-gimplify.c2019-11-15 11:23:50.574203292 +0100
@@ -35,6 +35,9 @@ along with GCC; see the file COPYING3.
#include "attribs.h"
#include "asan.h"
#include "gcc-rich-location.h"
+#include "output.h"
+#include "file-prefix-map.h"
+#include "cgraph.h"
/* Forward declarations. */
@@ -896,8 +899,12 @@ cp_gimplify_expr (tree *expr_p, gimple_s
tree decl = cp_get_callee_fndecl_nofold (*expr_p);
if (decl
&& fndecl_built_in_p (decl, CP_BUILT_IN_IS_CONSTANT_EVALUATED,
- BUILT_IN_FRONTEND))
+ BUILT_IN_FRONTEND))
*expr_p = boolean_false_node;
+ else if (decl
+ && fndecl_built_in_p (decl, CP_BUILT_IN_SOURCE_LOCATION,
+BUILT_IN_FRONTEND))
+ *expr_p = fold_builtin_source_location (EXPR_LOCATION (*expr_p));
}
break;
@@ -2641,9 +2648,17 @@ cp_fold (tree x)
/* Defer folding __builtin_is_constant_evaluated. */
if (callee
&& fndecl_built_in_p (callee, CP_BUILT_IN_IS_CONSTANT_EVALUATED,
- BUILT_IN_FRONTEND))
+ BUILT_IN_FRONTEND))
break;
+ if (callee
+ && fndecl_built_in_p (callee,
Re: [RFC C++ PATCH] __builtin_source_location ()
On Thu, Nov 14, 2019 at 10:15:21PM +, Jonathan Wakely wrote:
> > namespace std {
> > struct source_location {
> >struct __impl {
>
> Will this work if the library type is actually in an inline namespace,
> such as std::__8::source_location::__impl (as for
> --enable-symvers=gnu-versioned-namespace) or
> std::__v1::source_location::__impl (as it probably would be in
> libc++).
>
> If I'm reading the patch correctly, it would work fine, because
> qualified lookup for std::source_location would find that name even if
> it's really in some inline namespace.
I'd say so, but the unfinished testsuite coverage would need to cover it of
course.
> > const char *__file;
> > const char *__function;
> > unsigned int __line, __column;
> >};
> >const void *__ptr;
>
> If the magic type the compiler generates is declared in the header,
> then this member might as well be 'const __impl*'.
Yes, with the static_cast on the __builtin_source_location ()
result sure. I can't easily make it return const std::source_location::__impl*
though, because the initialization of the builtins happens early, before
is parsed.
And as it is a nested class, I think I can't predeclare it in the compiler.
If it would be std::__source_location_impl instead of
std::source_location::__impl, perhaps I could pretend there is
namespace std { struct __source_location_impl; }
but I bet that wouldn't work well with the inline namespaces.
So, is const void * return from the builtin ok?
Jakub
Re: [RFC C++ PATCH] __builtin_source_location ()
On 14/11/19 20:34 +0100, Jakub Jelinek wrote:
Hi!
The following WIP patch implements __builtin_source_location (),
which returns const void pointer to a std::source_location::__impl
struct that is required to contain __file, __function, __line and __column
fields, the first two with const char * type, the latter some integral type.
I don't have testcase coverage yet and the hash map to allow sharing of
VAR_DECLs with the same location is commented out both because it
doesn't compile for some reason and because hashing on location_t
is not enough, we probably need to hash on both location_t and fndecl,
as the baz case in the following shows.
Comments?
namespace std {
struct source_location {
struct __impl {
Will this work if the library type is actually in an inline namespace,
such as std::__8::source_location::__impl (as for
--enable-symvers=gnu-versioned-namespace) or
std::__v1::source_location::__impl (as it probably would be in
libc++).
If I'm reading the patch correctly, it would work fine, because
qualified lookup for std::source_location would find that name even if
it's really in some inline namespace.
const char *__file;
const char *__function;
unsigned int __line, __column;
};
const void *__ptr;
If the magic type the compiler generates is declared in the header,
then this member might as well be 'const __impl*'.
constexpr source_location () : __ptr (nullptr) {}
static consteval source_location
current (const void *__p = __builtin_source_location ()) {
source_location __ret;
__ret.__ptr = __p;
return __ret;
}
constexpr const char *file () const {
return static_cast (__ptr)->__file;
Not really relevant to your patch, but I'll say it here for the
benefit of others reading these mails ...
On IRC I suggested that the default constructor should set the __ptr
member to null, and these member functions should check for null, e.g.
if (__ptr) [[likely]]
return __ptr->__function;
else
return "";
The alternative is for the default constructor to call
__builtin_source_location() or refer to some static object in the
runtime library, but both options waste space. Adding a [[likely]]
branch to the accessors wastes no space and should only penalise users
who are misusing source_location by trying to get meaningful values
out of default constructed objects. If that's a bit slower I don't
care.
[RFC C++ PATCH] __builtin_source_location ()
Hi!
The following WIP patch implements __builtin_source_location (),
which returns const void pointer to a std::source_location::__impl
struct that is required to contain __file, __function, __line and __column
fields, the first two with const char * type, the latter some integral type.
I don't have testcase coverage yet and the hash map to allow sharing of
VAR_DECLs with the same location is commented out both because it
doesn't compile for some reason and because hashing on location_t
is not enough, we probably need to hash on both location_t and fndecl,
as the baz case in the following shows.
Comments?
namespace std {
struct source_location {
struct __impl {
const char *__file;
const char *__function;
unsigned int __line, __column;
};
const void *__ptr;
constexpr source_location () : __ptr (nullptr) {}
static consteval source_location
current (const void *__p = __builtin_source_location ()) {
source_location __ret;
__ret.__ptr = __p;
return __ret;
}
constexpr const char *file () const {
return static_cast (__ptr)->__file;
}
constexpr const char *function () const {
return static_cast (__ptr)->__function;
}
constexpr unsigned line () const {
return static_cast (__ptr)->__line;
}
constexpr unsigned column () const {
return static_cast (__ptr)->__column;
}
};
}
using namespace std;
consteval source_location
bar (const source_location x = source_location::current ())
{
return x;
}
void
foo (const char **p, unsigned *q)
{
constexpr source_location s = source_location::current ();
constexpr source_location t = bar ();
p[0] = s.file ();
p[1] = s.function ();
q[0] = s.line ();
q[1] = s.column ();
p[2] = t.file ();
p[3] = t.function ();
q[2] = t.line ();
q[3] = t.column ();
constexpr const char *r = s.file ();
}
template
constexpr source_location
baz ()
{
return source_location::current ();
}
constexpr source_location s1 = baz <0> ();
constexpr source_location s2 = baz <1> ();
const source_location *p1 =
const source_location *p2 =
--- gcc/cp/tree.c.jj2019-11-13 10:54:45.437045793 +0100
+++ gcc/cp/tree.c 2019-11-14 18:11:42.391213117 +0100
@@ -445,7 +445,9 @@ builtin_valid_in_constant_expr_p (const_
if (DECL_BUILT_IN_CLASS (decl) != BUILT_IN_NORMAL)
{
if (fndecl_built_in_p (decl, CP_BUILT_IN_IS_CONSTANT_EVALUATED,
- BUILT_IN_FRONTEND))
+BUILT_IN_FRONTEND)
+ || fndecl_built_in_p (decl, CP_BUILT_IN_SOURCE_LOCATION,
+ BUILT_IN_FRONTEND))
return true;
/* Not a built-in. */
return false;
--- gcc/cp/constexpr.c.jj 2019-11-13 10:54:45.426045960 +0100
+++ gcc/cp/constexpr.c 2019-11-14 18:26:40.691581038 +0100
@@ -1238,6 +1238,9 @@ cxx_eval_builtin_function_call (const co
return boolean_true_node;
}
+ if (fndecl_built_in_p (fun, CP_BUILT_IN_SOURCE_LOCATION, BUILT_IN_FRONTEND))
+return fold_builtin_source_location (EXPR_LOCATION (t));
+
/* Be permissive for arguments to built-ins; __builtin_constant_p should
return constant false for a non-constant argument. */
constexpr_ctx new_ctx = *ctx;
--- gcc/cp/name-lookup.c.jj 2019-11-13 10:54:45.495044911 +0100
+++ gcc/cp/name-lookup.c2019-11-14 18:38:30.765804391 +0100
@@ -5747,6 +5747,8 @@ get_std_name_hint (const char *name)
{"shared_lock", "", cxx14},
{"shared_mutex", "", cxx17},
{"shared_timed_mutex", "", cxx14},
+/* . */
+{"source_location", "", cxx2a},
/* . */
{"basic_stringbuf", "", cxx98},
{"basic_istringstream", "", cxx98},
--- gcc/cp/cp-gimplify.c.jj 2019-11-06 08:58:38.036473709 +0100
+++ gcc/cp/cp-gimplify.c2019-11-14 20:22:32.905068438 +0100
@@ -35,6 +35,9 @@ along with GCC; see the file COPYING3.
#include "attribs.h"
#include "asan.h"
#include "gcc-rich-location.h"
+#include "output.h"
+#include "file-prefix-map.h"
+#include "cgraph.h"
/* Forward declarations. */
@@ -896,8 +899,12 @@ cp_gimplify_expr (tree *expr_p, gimple_s
tree decl = cp_get_callee_fndecl_nofold (*expr_p);
if (decl
&& fndecl_built_in_p (decl, CP_BUILT_IN_IS_CONSTANT_EVALUATED,
- BUILT_IN_FRONTEND))
+ BUILT_IN_FRONTEND))
*expr_p = boolean_false_node;
+ else if (decl
+ && fndecl_built_in_p (decl, CP_BUILT_IN_SOURCE_LOCATION,
+BUILT_IN_FRONTEND))
+ *expr_p = fold_builtin_source_location (EXPR_LOCATION (*expr_p));
}
break;
@@ -2641,9 +2648,17 @@ cp_fold (tree x)
/* Defer folding __builtin_is_constant_evaluated. */
if (callee
&& fndecl_built_in_p (callee, CP_BUILT_IN_IS_CONSTANT_EVALUATED,
- BUILT_IN_FRONTEND))
+
