From 24205dc57f054cb6cf50f1ce9904229a1b9f4b06 Mon Sep 17 00:00:00 2001 From: Peter Pronai Date: Wed, 9 Jul 2025 11:06:34 +0000 Subject: [PATCH] Improve diagnostics contribution README.md The workflow's description wasn't clear enough, add nice concrete examples. Fixes #25929 internal issue. (Actually a small followup for it.) Issue: https://gitee.com/openharmony/arkcompiler_ets_frontend/issues/ICL7SQ Change-Id: I76d5cefc79f06762c28c079711c9ffe4659727a7 Signed-off-by: Peter Pronai --- ets2panda/util/diagnostic/README.md | 70 +++++++++++++++++++++++++---- 1 file changed, 61 insertions(+), 9 deletions(-) diff --git a/ets2panda/util/diagnostic/README.md b/ets2panda/util/diagnostic/README.md index 042c927578..df068c7f61 100644 --- a/ets2panda/util/diagnostic/README.md +++ b/ets2panda/util/diagnostic/README.md @@ -1,19 +1,71 @@ # Diagnostic YAMLs These files encode the various diagnostic messages that es2panda might emit. -## Adding new diagnostics -When adding a new diagnostic, adding them at the end *guarantees* conflicts between PRs. We avoid that by keeping the lists sorted by name. +## Quickstart +### Install deps +You might need to run `scripts/install-deps-ubuntu -i=test` if you haven't done so in a while. -Keeping them sorted still doesn't solve the issue of two PRs trying to add diagnostics with the same id. We mostly solve that by *not* choosing the numbers by hand, nor just incrementing them, but by generating them randomly. +Or you can run `pip install numpy ruamel.yaml`. -Both are achieved by running `ets_frontend/ets2panda/scripts/normalize_yaml somefile.yaml`, where `somefile.yaml` is the one you have added new messages to. +### Normalizing +After any YAML modification, run either of these: +```bash +# to normalize all diagnostic YAMLs: +ets_frontend/ets2panda/scripts/normalize_yaml --all +``` -You can also run `normalize_yaml --all` to normalize all diagnostic YAMLs. +```bash +# to only normalize somefile.yaml +ets_frontend/ets2panda/scripts/normalize_yaml somefile.yaml +``` -## Deleting diagnostics -To avoid accidentally re-using old ids, please move them into the `graveyard` list at the end of the file. These are forever forbidden from being emitted by the compiler. +### Adding new diagnostics +Add them before the graveyard, do not add an `id` field, then run `normalize_yaml`, as described above. + +```yaml +... +- name: YIELD_IN_GENERATOR_PARAM + id: 54 + message: "Yield is not allowed in generator parameters." + +- name: MY_NEW_DIAGNOSTIC + message: Oops, something happened + # *NO* `id` field! + +graveyard: +- 317 +# See ets_frontend/ets2panda/util/diagnostic/README.md before contributing. +``` + +### Deleting already committed diagnostics +Copy the `id` to the graveyard, then delete the diagnostic entry, then run `normalize_yaml` as described above. See `semantic.yaml` for an example of how it should look. -## Checking -YAMLs are checked at build time in `ets_frontend/ets2panda/util/diagnostic/diagnostic.rb`. \ No newline at end of file +```yaml +- name: YIELD_IN_GENERATOR_PARAM + id: 54 + message: "Yield is not allowed in generator parameters." + +# Removed diagnostic: +# - name: MY_NEW_DIAGNOSTIC +# id: 12345 +# message: Oops, something happened + +graveyard: +- ... +- 12345 # copied from the removed diagnostic +# See ets_frontend/ets2panda/util/diagnostic/README.md before contributing. +``` + +### Checking +YAMLs are checked at build time in `ets_frontend/ets2panda/util/diagnostic/diagnostic.rb`. + +## Background +Adding new diagnostics at the end of diagnostic YAMLs used to result in lots of Git conflicts, because everyone was trying to modify the same part of the same file. + +This first problem was mostly solved by sorting diagnostics by their name. + +That still left the problem of unique but human-friendly identifiers, which was (mostly) solved by generating IDs randomly. `normalize_yaml` has an explanation of the math behind this. + +The deleted ids were also not tracked, so the graveyard was added. It is also kept sorted to minimize conflicts and to make it easier to find ids. -- Gitee