[PATCH] Come up with json::integer_number and use it in GCOV.

[PATCH] Come up with json::integer_number and use it in GCOV.

[Martin Liška]

[-- 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;
 }
 

Re: [PATCH] Come up with json::integer_number and use it in GCOV.

[David Malcolm]

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.

> 

Re: [PATCH] Come up with json::integer_number and use it in GCOV.

[Martin Liška]

[-- 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

Re: [PATCH] Come up with json::integer_number and use it in GCOV.

[David Malcolm]

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

Re: [PATCH] Come up with json::integer_number and use it in GCOV.

[Martin Liška]

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

Re: [PATCH] Come up with json::integer_number and use it in GCOV.

[Martin Liška]

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
> 

Re: [PATCH] Come up with json::integer_number and use it in GCOV.

[Martin Liška]

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
>>
> 

Re: [PATCH] Come up with json::integer_number and use it in GCOV.

[Martin Liška]

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
>>>
>>
> 

Re: [PATCH] Come up with json::integer_number and use it in GCOV.

[Martin Liška]

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
>>>>
>>>
>>
> 

Re: [PATCH] Come up with json::integer_number and use it in GCOV.

[Jeff Law]

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