* [PATCH] Come up with json::integer_number and use it in GCOV. @ 2019-07-31 8:50 Martin Liška 2019-07-31 13:16 ` David Malcolm 0 siblings, 1 reply; 10+ messages in thread From: Martin Liška @ 2019-07-31 8:50 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm [-- Attachment #1: Type: text/plain, Size: 1887 bytes --] Hi. As seen here: https://github.com/RPGillespie6/fastcov/pull/18/files/f184dd8b6fc14e075dfc0ea93de7be5c96298ddb#r308735088 GCOV uses json::number for branch count, line count and similar. However, the json library uses a double as an internal representation for numbers. That's no desirable in case of GCOV and so that I would like to come up with integer_number type. David would you be fine with that? Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin gcc/ChangeLog: 2019-07-31 Martin Liska <mliska@suse.cz> * diagnostic-format-json.cc (json_from_expanded_location): Use json::float_number. * gcov.c (output_intermediate_json_line): Use new json::integer_number. (output_json_intermediate_file): Likewise. * json.cc (number::print): Move to ... (float_number::print): ... this. (integer_number::print): New. (test_writing_numbers): Move to ... (test_writing_float_numbers): ... this. (test_writing_integer_numbers): New. (json_cc_tests): Register test_writing_integer_numbers. * json.h (class value): Add forward declaration for float_number and integer_number. (enum kind): Add JSON_INTEGER and JSON_FLOAT. (class number): Move to ... (class float_number): ... this. (class integer_number): New. * optinfo-emit-json.cc (optrecord_json_writer::impl_location_to_json): Use json::float_number. (optrecord_json_writer::location_to_json): Likewise. (optrecord_json_writer::profile_count_to_json): Likewise. (optrecord_json_writer::pass_to_json): Likewise. --- gcc/diagnostic-format-json.cc | 4 ++-- gcc/gcov.c | 23 +++++++++++--------- gcc/json.cc | 41 ++++++++++++++++++++++++++++------- gcc/json.h | 35 ++++++++++++++++++++++++------ gcc/optinfo-emit-json.cc | 10 ++++----- 5 files changed, 81 insertions(+), 32 deletions(-) [-- Attachment #2: 0001-Come-up-with-json-integer_number-and-use-it-in-GCOV.patch --] [-- Type: text/x-patch, Size: 8906 bytes --] diff --git a/gcc/diagnostic-format-json.cc b/gcc/diagnostic-format-json.cc index 53c3b630b1c..2802da8a0a6 100644 --- a/gcc/diagnostic-format-json.cc +++ b/gcc/diagnostic-format-json.cc @@ -48,8 +48,8 @@ json_from_expanded_location (location_t loc) json::object *result = new json::object (); if (exploc.file) result->set ("file", new json::string (exploc.file)); - result->set ("line", new json::number (exploc.line)); - result->set ("column", new json::number (exploc.column)); + result->set ("line", new json::float_number (exploc.line)); + result->set ("column", new json::float_number (exploc.column)); return result; } diff --git a/gcc/gcov.c b/gcc/gcov.c index c65b7153765..ef006d125a2 100644 --- a/gcc/gcov.c +++ b/gcc/gcov.c @@ -1061,10 +1061,10 @@ output_intermediate_json_line (json::array *object, return; json::object *lineo = new json::object (); - lineo->set ("line_number", new json::number (line_num)); + lineo->set ("line_number", new json::integer_number (line_num)); if (function_name != NULL) lineo->set ("function_name", new json::string (function_name)); - lineo->set ("count", new json::number (line->count)); + lineo->set ("count", new json::integer_number (line->count)); lineo->set ("unexecuted_block", new json::literal (line->has_unexecuted_block)); @@ -1079,7 +1079,7 @@ output_intermediate_json_line (json::array *object, if (!(*it)->is_unconditional && !(*it)->is_call_non_return) { json::object *branch = new json::object (); - branch->set ("count", new json::number ((*it)->count)); + branch->set ("count", new json::integer_number ((*it)->count)); branch->set ("throw", new json::literal ((*it)->is_throw)); branch->set ("fallthrough", new json::literal ((*it)->fall_through)); @@ -1138,16 +1138,19 @@ output_json_intermediate_file (json::array *json_files, source_info *src) function->set ("name", new json::string ((*it)->m_name)); function->set ("demangled_name", new json::string ((*it)->get_demangled_name ())); - function->set ("start_line", new json::number ((*it)->start_line)); - function->set ("start_column", new json::number ((*it)->start_column)); - function->set ("end_line", new json::number ((*it)->end_line)); - function->set ("end_column", new json::number ((*it)->end_column)); + function->set ("start_line", + new json::integer_number ((*it)->start_line)); + function->set ("start_column", + new json::integer_number ((*it)->start_column)); + function->set ("end_line", new json::integer_number ((*it)->end_line)); + function->set ("end_column", + new json::integer_number ((*it)->end_column)); function->set ("blocks", - new json::number ((*it)->get_block_count ())); + new json::integer_number ((*it)->get_block_count ())); function->set ("blocks_executed", - new json::number ((*it)->blocks_executed)); + new json::integer_number ((*it)->blocks_executed)); function->set ("execution_count", - new json::number ((*it)->blocks[0].count)); + new json::integer_number ((*it)->blocks[0].count)); functions->append (function); } diff --git a/gcc/json.cc b/gcc/json.cc index 512e2e513b9..bec6fc53cc8 100644 --- a/gcc/json.cc +++ b/gcc/json.cc @@ -154,18 +154,31 @@ array::append (value *v) m_elements.safe_push (v); } -/* class json::number, a subclass of json::value, wrapping a double. */ +/* class json::float_number, a subclass of json::value, wrapping a double. */ -/* Implementation of json::value::print for json::number. */ +/* Implementation of json::value::print for json::float_number. */ void -number::print (pretty_printer *pp) const +float_number::print (pretty_printer *pp) const { char tmp[1024]; snprintf (tmp, sizeof (tmp), "%g", m_value); pp_string (pp, tmp); } +/* class json::integer_number, a subclass of json::value, wrapping a long. */ + +/* Implementation of json::value::print for json::integer_number. */ + +void +integer_number::print (pretty_printer *pp) const +{ + char tmp[1024]; + snprintf (tmp, sizeof (tmp), "%ld", m_value); + pp_string (pp, tmp); +} + + /* class json::string, a subclass of json::value. */ /* json::string's ctor. */ @@ -297,11 +310,22 @@ test_writing_arrays () /* Verify that JSON numbers are written correctly. */ static void -test_writing_numbers () +test_writing_float_numbers () +{ + assert_print_eq (float_number (0), "0"); + assert_print_eq (float_number (42), "42"); + assert_print_eq (float_number (-100), "-100"); + assert_print_eq (float_number (123456789), "1.23457e+08"); +} + +static void +test_writing_integer_numbers () { - assert_print_eq (number (0), "0"); - assert_print_eq (number (42), "42"); - assert_print_eq (number (-100), "-100"); + assert_print_eq (integer_number (0), "0"); + assert_print_eq (integer_number (42), "42"); + assert_print_eq (integer_number (-100), "-100"); + assert_print_eq (integer_number (123456789), "123456789"); + assert_print_eq (integer_number (-123456789), "-123456789"); } /* Verify that JSON strings are written correctly. */ @@ -337,7 +361,8 @@ json_cc_tests () test_object_get (); test_writing_objects (); test_writing_arrays (); - test_writing_numbers (); + test_writing_float_numbers (); + test_writing_integer_numbers (); test_writing_strings (); test_writing_literals (); } diff --git a/gcc/json.h b/gcc/json.h index d8a690ede5c..0f4904b37f3 100644 --- a/gcc/json.h +++ b/gcc/json.h @@ -39,7 +39,8 @@ namespace json class value; class object; class array; - class number; + class float_number; + class integer_number; class string; class literal; @@ -53,8 +54,11 @@ enum kind /* class json::array. */ JSON_ARRAY, - /* class json::number. */ - JSON_NUMBER, + /* class json::integer_number. */ + JSON_INTEGER, + + /* class json::float_number. */ + JSON_FLOAT, /* class json::string. */ JSON_STRING, @@ -114,14 +118,14 @@ class array : public value auto_vec<value *> m_elements; }; -/* Subclass of value for numbers. */ +/* Subclass of value for floats. */ -class number : public value +class float_number : public value { public: - number (double value) : m_value (value) {} + float_number (double value) : m_value (value) {} - enum kind get_kind () const FINAL OVERRIDE { return JSON_NUMBER; } + enum kind get_kind () const FINAL OVERRIDE { return JSON_FLOAT; } void print (pretty_printer *pp) const FINAL OVERRIDE; double get () const { return m_value; } @@ -130,6 +134,23 @@ class number : public value double m_value; }; +/* Subclass of value for integers. */ + +class integer_number : public value +{ + public: + integer_number (long value) : m_value (value) {} + + enum kind get_kind () const FINAL OVERRIDE { return JSON_INTEGER; } + void print (pretty_printer *pp) const FINAL OVERRIDE; + + long get () const { return m_value; } + + private: + long m_value; +}; + + /* Subclass of value for strings. */ class string : public value diff --git a/gcc/optinfo-emit-json.cc b/gcc/optinfo-emit-json.cc index 1cfcdfe8948..87cc940133a 100644 --- a/gcc/optinfo-emit-json.cc +++ b/gcc/optinfo-emit-json.cc @@ -181,7 +181,7 @@ optrecord_json_writer::impl_location_to_json (dump_impl_location_t loc) { json::object *obj = new json::object (); obj->set ("file", new json::string (loc.m_file)); - obj->set ("line", new json::number (loc.m_line)); + obj->set ("line", new json::float_number (loc.m_line)); if (loc.m_function) obj->set ("function", new json::string (loc.m_function)); return obj; @@ -196,8 +196,8 @@ optrecord_json_writer::location_to_json (location_t loc) expanded_location exploc = expand_location (loc); json::object *obj = new json::object (); obj->set ("file", new json::string (exploc.file)); - obj->set ("line", new json::number (exploc.line)); - obj->set ("column", new json::number (exploc.column)); + obj->set ("line", new json::float_number (exploc.line)); + obj->set ("column", new json::float_number (exploc.column)); return obj; } @@ -207,7 +207,7 @@ json::object * optrecord_json_writer::profile_count_to_json (profile_count count) { json::object *obj = new json::object (); - obj->set ("value", new json::number (count.to_gcov_type ())); + obj->set ("value", new json::float_number (count.to_gcov_type ())); obj->set ("quality", new json::string (profile_quality_as_string (count.quality ()))); return obj; @@ -262,7 +262,7 @@ optrecord_json_writer::pass_to_json (opt_pass *pass) && (pass->optinfo_flags & optgroup->value)) optgroups->append (new json::string (optgroup->name)); } - obj->set ("num", new json::number (pass->static_pass_number)); + obj->set ("num", new json::float_number (pass->static_pass_number)); return obj; } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Come up with json::integer_number and use it in GCOV. 2019-07-31 8:50 [PATCH] Come up with json::integer_number and use it in GCOV Martin Liška @ 2019-07-31 13:16 ` David Malcolm 2019-08-02 10:22 ` Martin Liška 0 siblings, 1 reply; 10+ messages in thread From: David Malcolm @ 2019-07-31 13:16 UTC (permalink / raw) To: Martin Liška, gcc-patches On Wed, 2019-07-31 at 10:42 +0200, Martin LiÅ¡ka wrote: > Hi. > > As seen here: > https://github.com/RPGillespie6/fastcov/pull/18/files/f184dd8b6fc14e0 > 75dfc0ea93de7be5c96298ddb#r308735088 > > GCOV uses json::number for branch count, line count and similar. > However, the json library > uses a double as an internal representation for numbers. That's no > desirable in case > of GCOV and so that I would like to come up with integer_number type. > David would you be fine with that? Thanks for the patch; overall I'm broadly in favor of the idea, but I think the patch needs a little work. The JSON standard has a definition for numbers that allows for both integers and floats, and says this about interoperability: https://tools.ietf.org/html/rfc7159#section-6 https://tools.ietf.org/html/rfc8259#section-6 "This specification allows implementations to set limits on the range and precision of numbers accepted. Since software that implements IEEE 754 binary64 (double precision) numbers [IEEE754] is generally available and widely used, good interoperability can be achieved by implementations that expect no more precision or range than these provide, in the sense that implementations will approximate JSON numbers within the expected precision." Hence I chose "double" as the representation. But, yeah, it's a pain when dealing with large integers. [see also "Parsing JSON is a Minefield" http://seriot.ch/parsing_json.php#22 ] Looking at your patch, you convert some places to integer_number, and some to float_number. It looks to me like you converted the gcov places you were concerned about to integer, and kept the "status quo" as floats for the other ones. But in all of the places I can see, I think integers make more sense than floats. I think we want to preserve the capability to emit floating point json values, but I suspect we want to use integer values everywhere we're currently emitting json; it's probably worth going through them line by line... > diff --git a/gcc/diagnostic-format-json.cc b/gcc/diagnostic-format-json.cc > index 53c3b630b1c..2802da8a0a6 100644 > --- a/gcc/diagnostic-format-json.cc > +++ b/gcc/diagnostic-format-json.cc > @@ -48,8 +48,8 @@ json_from_expanded_location (location_t loc) > json::object *result = new json::object (); > if (exploc.file) > result->set ("file", new json::string (exploc.file)); > - result->set ("line", new json::number (exploc.line)); > - result->set ("column", new json::number (exploc.column)); > + result->set ("line", new json::float_number (exploc.line)); > + result->set ("column", new json::float_number (exploc.column)); These should be integers. [...snip gcov hunks...] > diff --git a/gcc/json.cc b/gcc/json.cc > index 512e2e513b9..bec6fc53cc8 100644 > --- a/gcc/json.cc > +++ b/gcc/json.cc > @@ -154,18 +154,31 @@ array::append (value *v) > m_elements.safe_push (v); > } > > -/* class json::number, a subclass of json::value, wrapping a double. */ > +/* class json::float_number, a subclass of json::value, wrapping a double. */ Would it make more sense to call this "double_number"? (am not sure) > -/* Implementation of json::value::print for json::number. */ > +/* Implementation of json::value::print for json::float_number. */ > > void > -number::print (pretty_printer *pp) const > +float_number::print (pretty_printer *pp) const > { > char tmp[1024]; > snprintf (tmp, sizeof (tmp), "%g", m_value); > pp_string (pp, tmp); > } > > +/* class json::integer_number, a subclass of json::value, wrapping a long. */ Likewise, would it make more sense to call this "long"? An idea I had reading your patch: would a template be appropriate here, something like: template <typename T> class number : public value { enum kind get_kind () const FINAL OVERRIDE; T get () const { return m_value; } /* etc */ T m_value; }; with specializations for "double" and "long"? (hence json::number<double> json::number<long>, and enum values in the json_kind discriminator>). I think that a template might be overthinking things, though; the approach in your patch has the virtue of simplicity. Is "long" always wide enough for all the integer values we might want to express on the host? [...snip...] > -/* Subclass of value for numbers. */ > +/* Subclass of value for floats. */ I'd write "floating-point numbers" here (given that JSON standard talks about "numbers". [...] > +/* Subclass of value for integers. */ Likewise "integer-valued numbers" here, or somesuch. [...] > diff --git a/gcc/optinfo-emit-json.cc b/gcc/optinfo-emit-json.cc > index 1cfcdfe8948..87cc940133a 100644 > --- a/gcc/optinfo-emit-json.cc > +++ b/gcc/optinfo-emit-json.cc > @@ -181,7 +181,7 @@ optrecord_json_writer::impl_location_to_json (dump_impl_location_t loc) > { > json::object *obj = new json::object (); > obj->set ("file", new json::string (loc.m_file)); > - obj->set ("line", new json::number (loc.m_line)); > + obj->set ("line", new json::float_number (loc.m_line)); integer here, not float. > if (loc.m_function) > obj->set ("function", new json::string (loc.m_function)); > return obj; > @@ -196,8 +196,8 @@ optrecord_json_writer::location_to_json (location_t loc) > expanded_location exploc = expand_location (loc); > json::object *obj = new json::object (); > obj->set ("file", new json::string (exploc.file)); > - obj->set ("line", new json::number (exploc.line)); > - obj->set ("column", new json::number (exploc.column)); > + obj->set ("line", new json::float_number (exploc.line)); > + obj->set ("column", new json::float_number (exploc.column)); Likewise, integers here. > return obj; > } > > @@ -207,7 +207,7 @@ json::object * > optrecord_json_writer::profile_count_to_json (profile_count count) > { > json::object *obj = new json::object (); > - obj->set ("value", new json::number (count.to_gcov_type ())); > + obj->set ("value", new json::float_number (count.to_gcov_type ())); Presumably this should be an integer also. > obj->set ("quality", > new json::string (profile_quality_as_string (count.quality ()))); > return obj; > @@ -262,7 +262,7 @@ optrecord_json_writer::pass_to_json (opt_pass *pass) > && (pass->optinfo_flags & optgroup->value)) > optgroups->append (new json::string (optgroup->name)); > } > - obj->set ("num", new json::number (pass->static_pass_number)); > + obj->set ("num", new json::float_number (pass->static_pass_number)); Likewise. > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Come up with json::integer_number and use it in GCOV. 2019-07-31 13:16 ` David Malcolm @ 2019-08-02 10:22 ` Martin Liška 2019-08-02 12:40 ` David Malcolm 0 siblings, 1 reply; 10+ messages in thread From: Martin Liška @ 2019-08-02 10:22 UTC (permalink / raw) To: David Malcolm, gcc-patches [-- Attachment #1: Type: text/plain, Size: 7213 bytes --] On 7/31/19 3:13 PM, David Malcolm wrote: > On Wed, 2019-07-31 at 10:42 +0200, Martin LiÅ¡ka wrote: >> Hi. >> >> As seen here: >> https://github.com/RPGillespie6/fastcov/pull/18/files/f184dd8b6fc14e0 >> 75dfc0ea93de7be5c96298ddb#r308735088 >> >> GCOV uses json::number for branch count, line count and similar. >> However, the json library >> uses a double as an internal representation for numbers. That's no >> desirable in case >> of GCOV and so that I would like to come up with integer_number type. >> David would you be fine with that? > > Thanks for the patch; overall I'm broadly in favor of the idea, but I > think the patch needs a little work. > > The JSON standard has a definition for numbers that allows for both > integers and floats, and says this about interoperability: > > https://tools.ietf.org/html/rfc7159#section-6 > https://tools.ietf.org/html/rfc8259#section-6 > > "This specification allows implementations to set limits on the range > and precision of numbers accepted. Since software that implements > IEEE 754 binary64 (double precision) numbers [IEEE754] is generally > available and widely used, good interoperability can be achieved by > implementations that expect no more precision or range than these > provide, in the sense that implementations will approximate JSON > numbers within the expected precision." > > Hence I chose "double" as the representation. But, yeah, it's a pain > when dealing with large integers. > > [see also "Parsing JSON is a Minefield" > http://seriot.ch/parsing_json.php#22 ] > > Looking at your patch, you convert some places to integer_number, and > some to float_number. > > It looks to me like you converted the gcov places you were concerned > about to integer, and kept the "status quo" as floats for the other > ones. But in all of the places I can see, I think integers make more > sense than floats. Yep, but you are right that all other places also needed integer_type :) > > I think we want to preserve the capability to emit floating point json > values, but I suspect we want to use integer values everywhere we're > currently emitting json; it's probably worth going through them line by > line... I'm fine with preservation of the type. > >> diff --git a/gcc/diagnostic-format-json.cc b/gcc/diagnostic-format-json.cc >> index 53c3b630b1c..2802da8a0a6 100644 >> --- a/gcc/diagnostic-format-json.cc >> +++ b/gcc/diagnostic-format-json.cc >> @@ -48,8 +48,8 @@ json_from_expanded_location (location_t loc) >> json::object *result = new json::object (); >> if (exploc.file) >> result->set ("file", new json::string (exploc.file)); >> - result->set ("line", new json::number (exploc.line)); >> - result->set ("column", new json::number (exploc.column)); >> + result->set ("line", new json::float_number (exploc.line)); >> + result->set ("column", new json::float_number (exploc.column)); > > These should be integers. > > > [...snip gcov hunks...] > >> diff --git a/gcc/json.cc b/gcc/json.cc >> index 512e2e513b9..bec6fc53cc8 100644 >> --- a/gcc/json.cc >> +++ b/gcc/json.cc >> @@ -154,18 +154,31 @@ array::append (value *v) >> m_elements.safe_push (v); >> } >> >> -/* class json::number, a subclass of json::value, wrapping a double. */ >> +/* class json::float_number, a subclass of json::value, wrapping a double. */ > > Would it make more sense to call this "double_number"? (am not sure) I would prefer to stay with json::float_number. > > >> -/* Implementation of json::value::print for json::number. */ >> +/* Implementation of json::value::print for json::float_number. */ >> >> void >> -number::print (pretty_printer *pp) const >> +float_number::print (pretty_printer *pp) const >> { >> char tmp[1024]; >> snprintf (tmp, sizeof (tmp), "%g", m_value); >> pp_string (pp, tmp); >> } >> >> +/* class json::integer_number, a subclass of json::value, wrapping a long. */ > > Likewise, would it make more sense to call this "long"? > > An idea I had reading your patch: would a template be appropriate here, > something like: > > template <typename T> > class number : public value > { > enum kind get_kind () const FINAL OVERRIDE; > T get () const { return m_value; } > /* etc */ > > T m_value; > }; > > with specializations for "double" and "long"? > (hence json::number<double> json::number<long>, and enum values in the > json_kind discriminator>). > > I think that a template might be overthinking things, though; > the approach in your patch has the virtue of simplicity. > > Is "long" always wide enough for all the integer values we might want > to express on the host? Well, I would not over engineer it :) > > [...snip...] > >> -/* Subclass of value for numbers. */ >> +/* Subclass of value for floats. */ > > I'd write "floating-point numbers" here (given that JSON standard > talks about "numbers". Changed. > > [...] > >> +/* Subclass of value for integers. */ > > Likewise "integer-valued numbers" here, or somesuch. Changed. > > [...] > >> diff --git a/gcc/optinfo-emit-json.cc b/gcc/optinfo-emit-json.cc >> index 1cfcdfe8948..87cc940133a 100644 >> --- a/gcc/optinfo-emit-json.cc >> +++ b/gcc/optinfo-emit-json.cc >> @@ -181,7 +181,7 @@ optrecord_json_writer::impl_location_to_json (dump_impl_location_t loc) >> { >> json::object *obj = new json::object (); >> obj->set ("file", new json::string (loc.m_file)); >> - obj->set ("line", new json::number (loc.m_line)); >> + obj->set ("line", new json::float_number (loc.m_line)); > > integer here, not float. > >> if (loc.m_function) >> obj->set ("function", new json::string (loc.m_function)); >> return obj; >> @@ -196,8 +196,8 @@ optrecord_json_writer::location_to_json (location_t loc) >> expanded_location exploc = expand_location (loc); >> json::object *obj = new json::object (); >> obj->set ("file", new json::string (exploc.file)); >> - obj->set ("line", new json::number (exploc.line)); >> - obj->set ("column", new json::number (exploc.column)); >> + obj->set ("line", new json::float_number (exploc.line)); >> + obj->set ("column", new json::float_number (exploc.column)); > > Likewise, integers here. > >> return obj; >> } >> >> @@ -207,7 +207,7 @@ json::object * >> optrecord_json_writer::profile_count_to_json (profile_count count) >> { >> json::object *obj = new json::object (); >> - obj->set ("value", new json::number (count.to_gcov_type ())); >> + obj->set ("value", new json::float_number (count.to_gcov_type ())); > > Presumably this should be an integer also. > >> obj->set ("quality", >> new json::string (profile_quality_as_string (count.quality ()))); >> return obj; >> @@ -262,7 +262,7 @@ optrecord_json_writer::pass_to_json (opt_pass *pass) >> && (pass->optinfo_flags & optgroup->value)) >> optgroups->append (new json::string (optgroup->name)); >> } >> - obj->set ("num", new json::number (pass->static_pass_number)); >> + obj->set ("num", new json::float_number (pass->static_pass_number)); > > Likewise. > >> And I changed all occurrences of float_number with integer_number as you suggested. I'm currently testing the updated patch. Martin [-- Attachment #2: 0001-Come-up-with-json-integer_number-and-use-it-in-GCOV.patch --] [-- Type: text/x-patch, Size: 10549 bytes --] From 0863b4e326838fdd16c423bd0b8f59f1dfd7b0f0 Mon Sep 17 00:00:00 2001 From: Martin Liska <mliska@suse.cz> Date: Wed, 31 Jul 2019 09:51:28 +0200 Subject: [PATCH] Come up with json::integer_number and use it in GCOV. gcc/ChangeLog: 2019-07-31 Martin Liska <mliska@suse.cz> * diagnostic-format-json.cc (json_from_expanded_location): Use json::integer_number. * gcov.c (output_intermediate_json_line): Use new json::integer_number. (output_json_intermediate_file): Likewise. * json.cc (number::print): Move to ... (float_number::print): ... this. (integer_number::print): New. (test_writing_numbers): Move to ... (test_writing_float_numbers): ... this. (test_writing_integer_numbers): New. (json_cc_tests): Register test_writing_integer_numbers. * json.h (class value): Add forward declaration for float_number and integer_number. (enum kind): Add JSON_INTEGER and JSON_FLOAT. (class number): Move to ... (class float_number): ... this. (class integer_number): New. * optinfo-emit-json.cc (optrecord_json_writer::impl_location_to_json): Use json::integer_number. (optrecord_json_writer::location_to_json): Likewise. (optrecord_json_writer::profile_count_to_json): Likewise. (optrecord_json_writer::pass_to_json): Likewise. --- gcc/diagnostic-format-json.cc | 4 ++-- gcc/gcov.c | 23 +++++++++++--------- gcc/json.cc | 41 ++++++++++++++++++++++++++++------- gcc/json.h | 35 ++++++++++++++++++++++++------ gcc/optinfo-emit-json.cc | 10 ++++----- 5 files changed, 81 insertions(+), 32 deletions(-) diff --git a/gcc/diagnostic-format-json.cc b/gcc/diagnostic-format-json.cc index 53c3b630b1c..ae812174ad7 100644 --- a/gcc/diagnostic-format-json.cc +++ b/gcc/diagnostic-format-json.cc @@ -48,8 +48,8 @@ json_from_expanded_location (location_t loc) json::object *result = new json::object (); if (exploc.file) result->set ("file", new json::string (exploc.file)); - result->set ("line", new json::number (exploc.line)); - result->set ("column", new json::number (exploc.column)); + result->set ("line", new json::integer_number (exploc.line)); + result->set ("column", new json::integer_number (exploc.column)); return result; } diff --git a/gcc/gcov.c b/gcc/gcov.c index c65b7153765..ef006d125a2 100644 --- a/gcc/gcov.c +++ b/gcc/gcov.c @@ -1061,10 +1061,10 @@ output_intermediate_json_line (json::array *object, return; json::object *lineo = new json::object (); - lineo->set ("line_number", new json::number (line_num)); + lineo->set ("line_number", new json::integer_number (line_num)); if (function_name != NULL) lineo->set ("function_name", new json::string (function_name)); - lineo->set ("count", new json::number (line->count)); + lineo->set ("count", new json::integer_number (line->count)); lineo->set ("unexecuted_block", new json::literal (line->has_unexecuted_block)); @@ -1079,7 +1079,7 @@ output_intermediate_json_line (json::array *object, if (!(*it)->is_unconditional && !(*it)->is_call_non_return) { json::object *branch = new json::object (); - branch->set ("count", new json::number ((*it)->count)); + branch->set ("count", new json::integer_number ((*it)->count)); branch->set ("throw", new json::literal ((*it)->is_throw)); branch->set ("fallthrough", new json::literal ((*it)->fall_through)); @@ -1138,16 +1138,19 @@ output_json_intermediate_file (json::array *json_files, source_info *src) function->set ("name", new json::string ((*it)->m_name)); function->set ("demangled_name", new json::string ((*it)->get_demangled_name ())); - function->set ("start_line", new json::number ((*it)->start_line)); - function->set ("start_column", new json::number ((*it)->start_column)); - function->set ("end_line", new json::number ((*it)->end_line)); - function->set ("end_column", new json::number ((*it)->end_column)); + function->set ("start_line", + new json::integer_number ((*it)->start_line)); + function->set ("start_column", + new json::integer_number ((*it)->start_column)); + function->set ("end_line", new json::integer_number ((*it)->end_line)); + function->set ("end_column", + new json::integer_number ((*it)->end_column)); function->set ("blocks", - new json::number ((*it)->get_block_count ())); + new json::integer_number ((*it)->get_block_count ())); function->set ("blocks_executed", - new json::number ((*it)->blocks_executed)); + new json::integer_number ((*it)->blocks_executed)); function->set ("execution_count", - new json::number ((*it)->blocks[0].count)); + new json::integer_number ((*it)->blocks[0].count)); functions->append (function); } diff --git a/gcc/json.cc b/gcc/json.cc index 512e2e513b9..bec6fc53cc8 100644 --- a/gcc/json.cc +++ b/gcc/json.cc @@ -154,18 +154,31 @@ array::append (value *v) m_elements.safe_push (v); } -/* class json::number, a subclass of json::value, wrapping a double. */ +/* class json::float_number, a subclass of json::value, wrapping a double. */ -/* Implementation of json::value::print for json::number. */ +/* Implementation of json::value::print for json::float_number. */ void -number::print (pretty_printer *pp) const +float_number::print (pretty_printer *pp) const { char tmp[1024]; snprintf (tmp, sizeof (tmp), "%g", m_value); pp_string (pp, tmp); } +/* class json::integer_number, a subclass of json::value, wrapping a long. */ + +/* Implementation of json::value::print for json::integer_number. */ + +void +integer_number::print (pretty_printer *pp) const +{ + char tmp[1024]; + snprintf (tmp, sizeof (tmp), "%ld", m_value); + pp_string (pp, tmp); +} + + /* class json::string, a subclass of json::value. */ /* json::string's ctor. */ @@ -297,11 +310,22 @@ test_writing_arrays () /* Verify that JSON numbers are written correctly. */ static void -test_writing_numbers () +test_writing_float_numbers () +{ + assert_print_eq (float_number (0), "0"); + assert_print_eq (float_number (42), "42"); + assert_print_eq (float_number (-100), "-100"); + assert_print_eq (float_number (123456789), "1.23457e+08"); +} + +static void +test_writing_integer_numbers () { - assert_print_eq (number (0), "0"); - assert_print_eq (number (42), "42"); - assert_print_eq (number (-100), "-100"); + assert_print_eq (integer_number (0), "0"); + assert_print_eq (integer_number (42), "42"); + assert_print_eq (integer_number (-100), "-100"); + assert_print_eq (integer_number (123456789), "123456789"); + assert_print_eq (integer_number (-123456789), "-123456789"); } /* Verify that JSON strings are written correctly. */ @@ -337,7 +361,8 @@ json_cc_tests () test_object_get (); test_writing_objects (); test_writing_arrays (); - test_writing_numbers (); + test_writing_float_numbers (); + test_writing_integer_numbers (); test_writing_strings (); test_writing_literals (); } diff --git a/gcc/json.h b/gcc/json.h index d8a690ede5c..316bc8b254c 100644 --- a/gcc/json.h +++ b/gcc/json.h @@ -39,7 +39,8 @@ namespace json class value; class object; class array; - class number; + class float_number; + class integer_number; class string; class literal; @@ -53,8 +54,11 @@ enum kind /* class json::array. */ JSON_ARRAY, - /* class json::number. */ - JSON_NUMBER, + /* class json::integer_number. */ + JSON_INTEGER, + + /* class json::float_number. */ + JSON_FLOAT, /* class json::string. */ JSON_STRING, @@ -114,14 +118,14 @@ class array : public value auto_vec<value *> m_elements; }; -/* Subclass of value for numbers. */ +/* Subclass of value for floating-point numbers. */ -class number : public value +class float_number : public value { public: - number (double value) : m_value (value) {} + float_number (double value) : m_value (value) {} - enum kind get_kind () const FINAL OVERRIDE { return JSON_NUMBER; } + enum kind get_kind () const FINAL OVERRIDE { return JSON_FLOAT; } void print (pretty_printer *pp) const FINAL OVERRIDE; double get () const { return m_value; } @@ -130,6 +134,23 @@ class number : public value double m_value; }; +/* Subclass of value for integer-valued numbers. */ + +class integer_number : public value +{ + public: + integer_number (long value) : m_value (value) {} + + enum kind get_kind () const FINAL OVERRIDE { return JSON_INTEGER; } + void print (pretty_printer *pp) const FINAL OVERRIDE; + + long get () const { return m_value; } + + private: + long m_value; +}; + + /* Subclass of value for strings. */ class string : public value diff --git a/gcc/optinfo-emit-json.cc b/gcc/optinfo-emit-json.cc index 1cfcdfe8948..1ca4f148d15 100644 --- a/gcc/optinfo-emit-json.cc +++ b/gcc/optinfo-emit-json.cc @@ -181,7 +181,7 @@ optrecord_json_writer::impl_location_to_json (dump_impl_location_t loc) { json::object *obj = new json::object (); obj->set ("file", new json::string (loc.m_file)); - obj->set ("line", new json::number (loc.m_line)); + obj->set ("line", new json::integer_number (loc.m_line)); if (loc.m_function) obj->set ("function", new json::string (loc.m_function)); return obj; @@ -196,8 +196,8 @@ optrecord_json_writer::location_to_json (location_t loc) expanded_location exploc = expand_location (loc); json::object *obj = new json::object (); obj->set ("file", new json::string (exploc.file)); - obj->set ("line", new json::number (exploc.line)); - obj->set ("column", new json::number (exploc.column)); + obj->set ("line", new json::integer_number (exploc.line)); + obj->set ("column", new json::integer_number (exploc.column)); return obj; } @@ -207,7 +207,7 @@ json::object * optrecord_json_writer::profile_count_to_json (profile_count count) { json::object *obj = new json::object (); - obj->set ("value", new json::number (count.to_gcov_type ())); + obj->set ("value", new json::integer_number (count.to_gcov_type ())); obj->set ("quality", new json::string (profile_quality_as_string (count.quality ()))); return obj; @@ -262,7 +262,7 @@ optrecord_json_writer::pass_to_json (opt_pass *pass) && (pass->optinfo_flags & optgroup->value)) optgroups->append (new json::string (optgroup->name)); } - obj->set ("num", new json::number (pass->static_pass_number)); + obj->set ("num", new json::integer_number (pass->static_pass_number)); return obj; } -- 2.22.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Come up with json::integer_number and use it in GCOV. 2019-08-02 10:22 ` Martin Liška @ 2019-08-02 12:40 ` David Malcolm 2019-08-13 12:33 ` Martin Liška 0 siblings, 1 reply; 10+ messages in thread From: David Malcolm @ 2019-08-02 12:40 UTC (permalink / raw) To: Martin Liška, gcc-patches On Fri, 2019-08-02 at 12:22 +0200, Martin LiÅ¡ka wrote: > On 7/31/19 3:13 PM, David Malcolm wrote: > > On Wed, 2019-07-31 at 10:42 +0200, Martin LiÅ¡ka wrote: > > > Hi. > > > > > > As seen here: > > > https://github.com/RPGillespie6/fastcov/pull/18/files/f184dd8b6fc > > > 14e0 > > > 75dfc0ea93de7be5c96298ddb#r308735088 > > > > > > GCOV uses json::number for branch count, line count and similar. > > > However, the json library > > > uses a double as an internal representation for numbers. That's > > > no > > > desirable in case > > > of GCOV and so that I would like to come up with integer_number > > > type. > > > David would you be fine with that? > > > > Thanks for the patch; overall I'm broadly in favor of the idea, but > > I > > think the patch needs a little work. > > > > The JSON standard has a definition for numbers that allows for both > > integers and floats, and says this about interoperability: > > > > https://tools.ietf.org/html/rfc7159#section-6 > > https://tools.ietf.org/html/rfc8259#section-6 > > > > "This specification allows implementations to set limits on the > > range > > and precision of numbers accepted. Since software that > > implements > > IEEE 754 binary64 (double precision) numbers [IEEE754] is > > generally > > available and widely used, good interoperability can be achieved > > by > > implementations that expect no more precision or range than > > these > > provide, in the sense that implementations will approximate JSON > > numbers within the expected precision." > > > > Hence I chose "double" as the representation. But, yeah, it's a > > pain > > when dealing with large integers. > > > > [see also "Parsing JSON is a Minefield" > > http://seriot.ch/parsing_json.php#22 ] > > > > Looking at your patch, you convert some places to integer_number, > > and > > some to float_number. > > > > It looks to me like you converted the gcov places you were > > concerned > > about to integer, and kept the "status quo" as floats for the other > > ones. But in all of the places I can see, I think integers make > > more > > sense than floats. > > Yep, but you are right that all other places also needed integer_type > :) > > > > > I think we want to preserve the capability to emit floating point > > json > > values, but I suspect we want to use integer values everywhere > > we're > > currently emitting json; it's probably worth going through them > > line by > > line... > > I'm fine with preservation of the type. > > > > > > diff --git a/gcc/diagnostic-format-json.cc b/gcc/diagnostic- > > > format-json.cc > > > index 53c3b630b1c..2802da8a0a6 100644 > > > --- a/gcc/diagnostic-format-json.cc > > > +++ b/gcc/diagnostic-format-json.cc > > > @@ -48,8 +48,8 @@ json_from_expanded_location (location_t loc) > > > json::object *result = new json::object (); > > > if (exploc.file) > > > result->set ("file", new json::string (exploc.file)); > > > - result->set ("line", new json::number (exploc.line)); > > > - result->set ("column", new json::number (exploc.column)); > > > + result->set ("line", new json::float_number (exploc.line)); > > > + result->set ("column", new json::float_number > > > (exploc.column)); > > > > These should be integers. > > > > > > [...snip gcov hunks...] > > > > > diff --git a/gcc/json.cc b/gcc/json.cc > > > index 512e2e513b9..bec6fc53cc8 100644 > > > --- a/gcc/json.cc > > > +++ b/gcc/json.cc > > > @@ -154,18 +154,31 @@ array::append (value *v) > > > m_elements.safe_push (v); > > > } > > > > > > -/* class json::number, a subclass of json::value, wrapping a > > > double. */ > > > +/* class json::float_number, a subclass of json::value, wrapping > > > a double. */ > > > > Would it make more sense to call this "double_number"? (am not > > sure) > > I would prefer to stay with json::float_number. Fair enough. > > > > > > > -/* Implementation of json::value::print for json::number. */ > > > +/* Implementation of json::value::print for > > > json::float_number. */ > > > > > > void > > > -number::print (pretty_printer *pp) const > > > +float_number::print (pretty_printer *pp) const > > > { > > > char tmp[1024]; > > > snprintf (tmp, sizeof (tmp), "%g", m_value); > > > pp_string (pp, tmp); > > > } > > > > > > +/* class json::integer_number, a subclass of json::value, > > > wrapping a long. */ > > > > Likewise, would it make more sense to call this "long"? > > > > An idea I had reading your patch: would a template be appropriate > > here, > > something like: > > > > template <typename T> > > class number : public value > > { > > enum kind get_kind () const FINAL OVERRIDE; > > T get () const { return m_value; } > > /* etc */ > > > > T m_value; > > }; > > > > with specializations for "double" and "long"? > > (hence json::number<double> json::number<long>, and enum values in > > the > > json_kind discriminator>). > > > > I think that a template might be overthinking things, though; > > the approach in your patch has the virtue of simplicity. > > > > Is "long" always wide enough for all the integer values we might > > want > > to express on the host? > > Well, I would not over engineer it :) Likewise, fair enough. > > > > [...snip...] > > > > > -/* Subclass of value for numbers. */ > > > +/* Subclass of value for floats. */ > > > > I'd write "floating-point numbers" here (given that JSON standard > > talks about "numbers". > > Changed. > > > > > [...] > > > > > +/* Subclass of value for integers. */ > > > > Likewise "integer-valued numbers" here, or somesuch. > > Changed. > > > > > [...] > > > > > diff --git a/gcc/optinfo-emit-json.cc b/gcc/optinfo-emit-json.cc > > > index 1cfcdfe8948..87cc940133a 100644 > > > --- a/gcc/optinfo-emit-json.cc > > > +++ b/gcc/optinfo-emit-json.cc > > > @@ -181,7 +181,7 @@ optrecord_json_writer::impl_location_to_json > > > (dump_impl_location_t loc) > > > { > > > json::object *obj = new json::object (); > > > obj->set ("file", new json::string (loc.m_file)); > > > - obj->set ("line", new json::number (loc.m_line)); > > > + obj->set ("line", new json::float_number (loc.m_line)); > > > > integer here, not float. > > > > > if (loc.m_function) > > > obj->set ("function", new json::string (loc.m_function)); > > > return obj; > > > @@ -196,8 +196,8 @@ optrecord_json_writer::location_to_json > > > (location_t loc) > > > expanded_location exploc = expand_location (loc); > > > json::object *obj = new json::object (); > > > obj->set ("file", new json::string (exploc.file)); > > > - obj->set ("line", new json::number (exploc.line)); > > > - obj->set ("column", new json::number (exploc.column)); > > > + obj->set ("line", new json::float_number (exploc.line)); > > > + obj->set ("column", new json::float_number (exploc.column)); > > > > Likewise, integers here. > > > > > return obj; > > > } > > > > > > @@ -207,7 +207,7 @@ json::object * > > > optrecord_json_writer::profile_count_to_json (profile_count > > > count) > > > { > > > json::object *obj = new json::object (); > > > - obj->set ("value", new json::number (count.to_gcov_type ())); > > > + obj->set ("value", new json::float_number (count.to_gcov_type > > > ())); > > > > Presumably this should be an integer also. > > > > > obj->set ("quality", > > > new json::string (profile_quality_as_string > > > (count.quality ()))); > > > return obj; > > > @@ -262,7 +262,7 @@ optrecord_json_writer::pass_to_json (opt_pass > > > *pass) > > > && (pass->optinfo_flags & optgroup->value)) > > > optgroups->append (new json::string (optgroup->name)); > > > } > > > - obj->set ("num", new json::number (pass->static_pass_number)); > > > + obj->set ("num", new json::float_number (pass- > > > >static_pass_number)); > > > > Likewise. Something that occurred to me reading the updated patch: maybe it would make things easier to have utility member functions of json::object to implicitly make the child, e.g.: void json::object::set (const char *key, long v) { set (key, new json::integer_number (v)); } so that all those calls can be just: obj->set ("line", exploc.line); obj->set ("column", exploc.column); etc (assuming overloading is unambiguous). But that's probably orthogonal to this patch. > And I changed all occurrences of float_number with integer_number > as you suggested. Thanks. > I'm currently testing the updated patch. > Martin The updated patch looks good to me, but technically I'm not a reviewer for these files. Dave ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Come up with json::integer_number and use it in GCOV. 2019-08-02 12:40 ` David Malcolm @ 2019-08-13 12:33 ` Martin Liška 2019-08-26 13:06 ` Martin Liška 0 siblings, 1 reply; 10+ messages in thread From: Martin Liška @ 2019-08-13 12:33 UTC (permalink / raw) To: David Malcolm, gcc-patches; +Cc: Jakub Jelinek, Richard Biener On 8/2/19 2:40 PM, David Malcolm wrote: > Something that occurred to me reading the updated patch: maybe it would > make things easier to have utility member functions of json::object to > implicitly make the child, e.g.: > > void > json::object::set (const char *key, long v) > { > set (key, new json::integer_number (v)); > } > > so that all those calls can be just: > > obj->set ("line", exploc.line); > obj->set ("column", exploc.column); > > etc (assuming overloading is unambiguous). > > But that's probably orthogonal to this patch. Looks good to me. It's a candidate for a follow up patch. > > >> And I changed all occurrences of float_number with integer_number >> as you suggested. > Thanks. > >> I'm currently testing the updated patch. >> Martin > The updated patch looks good to me, but technically I'm not a reviewer > for these files. Sure, I hope @Jakub or @Richi can approve me that? Thanks, Martin > > Dave ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Come up with json::integer_number and use it in GCOV. 2019-08-13 12:33 ` Martin Liška @ 2019-08-26 13:06 ` Martin Liška 2019-08-30 9:08 ` Martin Liška 0 siblings, 1 reply; 10+ messages in thread From: Martin Liška @ 2019-08-26 13:06 UTC (permalink / raw) To: David Malcolm, gcc-patches; +Cc: Jakub Jelinek, Richard Biener PING^1 On 8/13/19 1:51 PM, Martin LiÅ¡ka wrote: > On 8/2/19 2:40 PM, David Malcolm wrote: >> Something that occurred to me reading the updated patch: maybe it would >> make things easier to have utility member functions of json::object to >> implicitly make the child, e.g.: >> >> void >> json::object::set (const char *key, long v) >> { >> set (key, new json::integer_number (v)); >> } >> >> so that all those calls can be just: >> >> obj->set ("line", exploc.line); >> obj->set ("column", exploc.column); >> >> etc (assuming overloading is unambiguous). >> >> But that's probably orthogonal to this patch. > > Looks good to me. It's a candidate for a follow up patch. > >> >> >>> And I changed all occurrences of float_number with integer_number >>> as you suggested. >> Thanks. >> >>> I'm currently testing the updated patch. >>> Martin >> The updated patch looks good to me, but technically I'm not a reviewer >> for these files. > > Sure, I hope @Jakub or @Richi can approve me that? > Thanks, > Martin > >> >> Dave > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Come up with json::integer_number and use it in GCOV. 2019-08-26 13:06 ` Martin Liška @ 2019-08-30 9:08 ` Martin Liška 2019-09-09 12:38 ` Martin Liška 0 siblings, 1 reply; 10+ messages in thread From: Martin Liška @ 2019-08-30 9:08 UTC (permalink / raw) To: David Malcolm, gcc-patches; +Cc: Jakub Jelinek, Richard Biener PING^2 On 8/26/19 2:34 PM, Martin LiÅ¡ka wrote: > PING^1 > > On 8/13/19 1:51 PM, Martin LiÅ¡ka wrote: >> On 8/2/19 2:40 PM, David Malcolm wrote: >>> Something that occurred to me reading the updated patch: maybe it would >>> make things easier to have utility member functions of json::object to >>> implicitly make the child, e.g.: >>> >>> void >>> json::object::set (const char *key, long v) >>> { >>> set (key, new json::integer_number (v)); >>> } >>> >>> so that all those calls can be just: >>> >>> obj->set ("line", exploc.line); >>> obj->set ("column", exploc.column); >>> >>> etc (assuming overloading is unambiguous). >>> >>> But that's probably orthogonal to this patch. >> >> Looks good to me. It's a candidate for a follow up patch. >> >>> >>> >>>> And I changed all occurrences of float_number with integer_number >>>> as you suggested. >>> Thanks. >>> >>>> I'm currently testing the updated patch. >>>> Martin >>> The updated patch looks good to me, but technically I'm not a reviewer >>> for these files. >> >> Sure, I hope @Jakub or @Richi can approve me that? >> Thanks, >> Martin >> >>> >>> Dave >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Come up with json::integer_number and use it in GCOV. 2019-08-30 9:08 ` Martin Liška @ 2019-09-09 12:38 ` Martin Liška 2019-09-18 8:04 ` Martin Liška 0 siblings, 1 reply; 10+ messages in thread From: Martin Liška @ 2019-09-09 12:38 UTC (permalink / raw) To: David Malcolm, gcc-patches; +Cc: Jakub Jelinek, Richard Biener PING^3 On 8/30/19 10:55 AM, Martin LiÅ¡ka wrote: > PING^2 > > On 8/26/19 2:34 PM, Martin LiÅ¡ka wrote: >> PING^1 >> >> On 8/13/19 1:51 PM, Martin LiÅ¡ka wrote: >>> On 8/2/19 2:40 PM, David Malcolm wrote: >>>> Something that occurred to me reading the updated patch: maybe it would >>>> make things easier to have utility member functions of json::object to >>>> implicitly make the child, e.g.: >>>> >>>> void >>>> json::object::set (const char *key, long v) >>>> { >>>> set (key, new json::integer_number (v)); >>>> } >>>> >>>> so that all those calls can be just: >>>> >>>> obj->set ("line", exploc.line); >>>> obj->set ("column", exploc.column); >>>> >>>> etc (assuming overloading is unambiguous). >>>> >>>> But that's probably orthogonal to this patch. >>> >>> Looks good to me. It's a candidate for a follow up patch. >>> >>>> >>>> >>>>> And I changed all occurrences of float_number with integer_number >>>>> as you suggested. >>>> Thanks. >>>> >>>>> I'm currently testing the updated patch. >>>>> Martin >>>> The updated patch looks good to me, but technically I'm not a reviewer >>>> for these files. >>> >>> Sure, I hope @Jakub or @Richi can approve me that? >>> Thanks, >>> Martin >>> >>>> >>>> Dave >>> >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Come up with json::integer_number and use it in GCOV. 2019-09-09 12:38 ` Martin Liška @ 2019-09-18 8:04 ` Martin Liška 2019-10-01 19:52 ` Jeff Law 0 siblings, 1 reply; 10+ messages in thread From: Martin Liška @ 2019-09-18 8:04 UTC (permalink / raw) To: David Malcolm, gcc-patches; +Cc: Jakub Jelinek, Richard Biener PING^4 Just note that the author of the JSON implementation in GCC is fine with the patch ;) Martin On 9/9/19 2:38 PM, Martin LiÅ¡ka wrote: > PING^3 > > On 8/30/19 10:55 AM, Martin LiÅ¡ka wrote: >> PING^2 >> >> On 8/26/19 2:34 PM, Martin LiÅ¡ka wrote: >>> PING^1 >>> >>> On 8/13/19 1:51 PM, Martin LiÅ¡ka wrote: >>>> On 8/2/19 2:40 PM, David Malcolm wrote: >>>>> Something that occurred to me reading the updated patch: maybe it would >>>>> make things easier to have utility member functions of json::object to >>>>> implicitly make the child, e.g.: >>>>> >>>>> void >>>>> json::object::set (const char *key, long v) >>>>> { >>>>> set (key, new json::integer_number (v)); >>>>> } >>>>> >>>>> so that all those calls can be just: >>>>> >>>>> obj->set ("line", exploc.line); >>>>> obj->set ("column", exploc.column); >>>>> >>>>> etc (assuming overloading is unambiguous). >>>>> >>>>> But that's probably orthogonal to this patch. >>>> >>>> Looks good to me. It's a candidate for a follow up patch. >>>> >>>>> >>>>> >>>>>> And I changed all occurrences of float_number with integer_number >>>>>> as you suggested. >>>>> Thanks. >>>>> >>>>>> I'm currently testing the updated patch. >>>>>> Martin >>>>> The updated patch looks good to me, but technically I'm not a reviewer >>>>> for these files. >>>> >>>> Sure, I hope @Jakub or @Richi can approve me that? >>>> Thanks, >>>> Martin >>>> >>>>> >>>>> Dave >>>> >>> >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Come up with json::integer_number and use it in GCOV. 2019-09-18 8:04 ` Martin Liška @ 2019-10-01 19:52 ` Jeff Law 0 siblings, 0 replies; 10+ messages in thread From: Jeff Law @ 2019-10-01 19:52 UTC (permalink / raw) To: Martin Liška, David Malcolm, gcc-patches Cc: Jakub Jelinek, Richard Biener On 9/18/19 2:04 AM, Martin LiÅ¡ka wrote: > PING^4 > > Just note that the author of the JSON implementation > in GCC is fine with the patch ;) OK if this is still pending :-) jeff ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-10-01 19:52 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-31 8:50 [PATCH] Come up with json::integer_number and use it in GCOV Martin Liška 2019-07-31 13:16 ` David Malcolm 2019-08-02 10:22 ` Martin Liška 2019-08-02 12:40 ` David Malcolm 2019-08-13 12:33 ` Martin Liška 2019-08-26 13:06 ` Martin Liška 2019-08-30 9:08 ` Martin Liška 2019-09-09 12:38 ` Martin Liška 2019-09-18 8:04 ` Martin Liška 2019-10-01 19:52 ` Jeff Law
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).