On Sat, 2026-02-14 at 16:46 -0800, Andrew Pinski wrote:
> On Sat, Feb 14, 2026 at 7:55 AM David Malcolm <[email protected]>
> wrote:
> > 
> > Use json-diagnostic.{h,cc} to improve the diagnostics issued
> > for malformed and invalid JSON tuning inputs: report the
> > file/line/column and, if possible, the JSON Pointer of the
> > problematic input.
> > 
> > Before
> > ------
> > 
> > $ ./xgcc -B. -S test.c -muser-provided-CPU=malformed.json
> > cc1: error: error parsing JSON data: expected ':'; got number
> > 
> > $ ./xgcc -B. -S test.c -muser-provided-CPU=unsigned-3.json
> > cc1: warning: JSON tuning file does not contain version
> > information; compatibility cannot be verified
> > cc1: error: key ‘tune_params.sve_width’ value 5000000000 is out of
> > range for ‘uint’ type [0, 4294967295]
> > cc1: error: validation failed for the provided JSON data
> > 
> > After
> > -----
> > 
> > $ ./xgcc -B. -S test.c -muser-provided-CPU=malformed.json
> > malformed.json:3:17: error: error parsing JSON data: expected ':';
> > got number
> >     3 |     "sve_width" 128
> >       |                 ^~~
> > 
> > $ ./xgcc -B. -S test.c -muser-provided-CPU=unsigned-3.json
> > cc1: warning: JSON tuning file does not contain version
> > information; compatibility cannot be verified
> > unsigned-3.json: In JSON value ‘/tune_params/sve_width’
> > unsigned-3.json:3:18: error: key ‘tune_params.sve_width’ value
> > 5000000000 is out of range for ‘uint’ type [0, 4294967295]
> >     3 |     "sve_width": 5000000000
> >       |                  ^~~~~~~~~~
> > cc1: error: validation failed for the provided JSON data
> > 
> > gcc/ChangeLog:
> >         PR target/124094
> >         * config/aarch64/aarch64-generate-json-tuning-routines.py
> >         (generate_field_code): Add "ctxt, " arg to function call
> > when
> >         operation is "parse".
> >         (generate_function): Add "gcc_json_context &ctxt, " param
> > to
> >         function decl when operation is "parse".
> >         * config/aarch64/aarch64-json-tunings-parser-generated.inc:
> >         Regenerate, passing around a gcc_json_context &.
> >         * config/aarch64/aarch64-json-tunings-parser.cc: Include
> >         "json-diagnostic.h".
> >         (WARNING_OPT) New macro.
> >         (PARSE_INTEGER_FIELD): Add "ctxt" param and pass it around.
> >         (PARSE_UNSIGNED_INTEGER_FIELD): Likewise.
> >         (PARSE_BOOLEAN_FIELD): Likewise.
> >         (PARSE_STRING_FIELD): Likewise.
> >         (PARSE_OBJECT): Likewise.
> >         (PARSE_ARRAY_FIELD): Likewise.
> >         (PARSE_ENUM_FIELD): Likewise.
> >         (parse_func_type): Likewise.
> >         (parse_object_helper): Likewise.  Use json_error rather
> > than error.
> >         (inform_about_wrong_kind_of_json_value): New.
> >         (extract_string): Add "ctxt" param.  Replace "warning" with
> > a pair
> >         of calls to json_warning and
> >         inform_about_wrong_kind_of_json_value, tweaking wording
> >         accordingly.
> >         (extract_integer): Likewise.
> >         (extract_unsigned_integer): Likewise.
> >         (parse_enum_field): Likewise.
> >         (validate_and_traverse): Replace "warning" and "error" with
> >         "json_warning" and "json_error".
> >         (check_version_compatibility): Use WARNING_OPT.  Add
> >         auto_diagnostic_group to group the error and note.
> >         (aarch64_load_tuning_params_from_json_string): Add
> > "js_filename"
> >         param.  Use gcc_json_context to capture location info.  Use
> > it
> >         when reporting errors to get file/line/column info. 
> > Replace check
> >         on root being non-null with assertion, as this is
> > guaranteed if
> >         error is non-null.  Replace warning with json_warning.
> >         (aarch64_load_tuning_params_from_json): Pass data_filename
> > to
> >         aarch64_load_tuning_params_from_json_string for use when
> > reporting
> >         diagnostics.
> >         (selftest::test_json_integers): Add a placeholder filename.
> >         (selftest::test_json_boolean): Likewise.
> >         (selftest::test_json_strings): Likewise.
> >         (selftest::test_json_enums): Likewise.
> > 
> > gcc/testsuite/ChangeLog:
> >         PR target/124094
> >         * gcc.target/aarch64/aarch64-json-tunings/boolean-2.c: Add
> > options
> >         -fdiagnostics-show-caret -fdiagnostics-show-line-numbers. 
> > Replace
> >         dg-error with a pair of dg-regexps to verify that we report
> > the
> >         filename and JSON Pointer of where the error occurs, and
> > then
> >         the filename and location within the JSON file.  Verify
> > that we
> >         quote and underline the pertinent part of the JSON file.
> >         * gcc.target/aarch64/aarch64-json-tunings/empty-brackets.c:
> > Likewise.
> >         * gcc.target/aarch64/aarch64-json-tunings/enum-2.c:
> > Likewise.
> >         * gcc.target/aarch64/aarch64-json-tunings/integer-2.c:
> > Likewise.
> >         * gcc.target/aarch64/aarch64-json-tunings/integer-3.c:
> > Likewise.
> >         * gcc.target/aarch64/aarch64-json-tunings/string-2.c:
> > Likewise.
> >         * gcc.target/aarch64/aarch64-json-tunings/unidentified-
> > key.c: Likewise.
> >         * gcc.target/aarch64/aarch64-json-tunings/unsigned-2.c:
> > Likewise.
> >         * gcc.target/aarch64/aarch64-json-tunings/unsigned-3.c:
> > Likewise.
> >         * gcc.target/aarch64/aarch64-json-tunings/malformed.c: New
> > test,
> >         to verify behavior on a malformed JSON input file.
> >         * gcc.target/aarch64/aarch64-json-tunings/malformed.json:
> > New
> >         test file.  This is malformed JSON, due to a missing ':"
> > between
> >         key and value.
> > 
> > Signed-off-by: David Malcolm <[email protected]>
> > ---
> >  .../aarch64-generate-json-tuning-routines.py  |  17 +-
> >  .../aarch64-json-tunings-parser-generated.inc | 428 +++++++++-----
> > ----
> >  .../aarch64/aarch64-json-tunings-parser.cc    | 276 +++++++----
> >  .../aarch64/aarch64-json-tunings/boolean-2.c  |  12 +-
> >  .../aarch64-json-tunings/empty-brackets.c     |   9 +-
> >  .../aarch64/aarch64-json-tunings/enum-2.c     |  19 +-
> >  .../aarch64/aarch64-json-tunings/integer-2.c  |  14 +-
> >  .../aarch64/aarch64-json-tunings/integer-3.c  |  13 +-
> >  .../aarch64/aarch64-json-tunings/malformed.c  |  11 +
> >  .../aarch64-json-tunings/malformed.json       |   5 +
> >  .../aarch64/aarch64-json-tunings/string-2.c   |  12 +-
> >  .../aarch64-json-tunings/unidentified-key.c   |  12 +-
> >  .../aarch64/aarch64-json-tunings/unsigned-2.c |  10 +-
> >  .../aarch64/aarch64-json-tunings/unsigned-3.c |  10 +-
> >  14 files changed, 517 insertions(+), 331 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.target/aarch64/aarch64-json-
> > tunings/malformed.c
> >  create mode 100644 gcc/testsuite/gcc.target/aarch64/aarch64-json-
> > tunings/malformed.json
> > 
> > diff --git a/gcc/config/aarch64/aarch64-generate-json-tuning-
> > routines.py b/gcc/config/aarch64/aarch64-generate-json-tuning-
> > routines.py
> > index a4f9e4ece71a3..9253cf58cdfa1 100755
> > --- a/gcc/config/aarch64/aarch64-generate-json-tuning-routines.py
> > +++ b/gcc/config/aarch64/aarch64-generate-json-tuning-routines.py
> > @@ -85,15 +85,20 @@ def generate_field_code(
> >  ) -> List[str]:
> >      lines = []
> > 
> > +    if operation == 'parse':
> > +        ctxt = 'ctxt, '
> > +    else:
> > +        ctxt = ''
> 
> What about passing around a ctxt pointer aways and then for the non
> parse case just pass a null pointer?
> Just in case we need a context later on for the output case and don't
> need to change this part again.

"ctxt" has responsibility for providing location_t values for
json::value * in any diagnostics.  Given that I want to rely on ctxt
being non-null I prefer to pass it around as a reference, and not
provide it at all for the generation case, where it currently serves no
purpose.

I think the YAGNI principle applies here.

> 
> Otherwise this looks ok to me.

Thanks.
Dave


> 
> Thanks,
> Andrew Pinski



Reply via email to