Conversation
See: #629 Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
🤖 Augment PR SummarySummary: Speeds up linting by avoiding work on schema locations/references that cannot be reached during validation. Changes:
Technical Notes: 🤖 Was this summary useful? React with 👍 or 👎 |
|
|
||
| SchemaFrame(const Mode mode) : mode_{mode} {} | ||
|
|
||
| // We rely on internal caches that would be dangling otherwise |
There was a problem hiding this comment.
| auto SchemaFrame::populate_reachability(const SchemaWalker &walker, | ||
| const SchemaResolver &resolver) const | ||
| -> void { | ||
| if (!this->reachability_.empty()) { |
There was a problem hiding this comment.
populate_reachability() memoizes into reachability_ without considering the walker/resolver arguments, so is_reachable() could return stale results if a SchemaFrame is queried with different resolver/walker instances. Is the intended contract that SchemaFrame is only ever used with the same walker/resolver (e.g., the ones used during analyse())?
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| return this->data < other.get().data; | ||
| } | ||
|
|
||
| /// Hash functor for use with containers |
There was a problem hiding this comment.
With the hash implementation now living in GenericPointer::Hasher, dropping the std::hash<GenericPointer<...>> specialization can be a breaking change for downstream users of unordered_*<Pointer> that relied on the default hasher. If this is intended, it may be worth explicitly documenting/calling out in release notes.
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| continue; | ||
| } | ||
|
|
||
| auto reference_origin{frame.traverse(reference.first.second)}; |
There was a problem hiding this comment.
This code assumes frame.traverse(reference.first.second) (and the parent traversals) can never return std::nullopt; in NDEBUG builds the assert()s compile out and reference_origin->get() would be UB if the invariant is violated. If unresolved/missing locations are possible here, consider guarding the optional before dereferencing.
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [sourcemeta/jsonschema](https://github.com/sourcemeta/jsonschema) | minor | `v14.5.0` → `v14.10.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>sourcemeta/jsonschema (sourcemeta/jsonschema)</summary> ### [`v14.10.0`](https://github.com/sourcemeta/jsonschema/releases/tag/v14.10.0) [Compare Source](sourcemeta/jsonschema@v14.9.0...v14.10.0) #### What's Changed - Implement a `--frozen` option for the `install` command by [@​jviotti](https://github.com/jviotti) in [#​656](sourcemeta/jsonschema#656) **Full Changelog**: <sourcemeta/jsonschema@v14.9.0...v14.10.0> ### [`v14.9.0`](https://github.com/sourcemeta/jsonschema/releases/tag/v14.9.0) [Compare Source](sourcemeta/jsonschema@v14.8.0...v14.9.0) #### What's Changed - Support adding new dependencies in `install` by [@​jviotti](https://github.com/jviotti) in [#​655](sourcemeta/jsonschema#655) **Full Changelog**: <sourcemeta/jsonschema@v14.8.0...v14.9.0> ### [`v14.8.0`](https://github.com/sourcemeta/jsonschema/releases/tag/v14.8.0) [Compare Source](sourcemeta/jsonschema@v14.7.2...v14.8.0) #### What's Changed - Implement an `install` command by [@​jviotti](https://github.com/jviotti) in [#​654](sourcemeta/jsonschema#654) - Note: this is a first take on the problem. More improvements coming very soon! **Full Changelog**: <sourcemeta/jsonschema@v14.7.2...v14.8.0> ### [`v14.7.2`](https://github.com/sourcemeta/jsonschema/releases/tag/v14.7.2) [Compare Source](sourcemeta/jsonschema@v14.7.1...v14.7.2) #### What's Changed - Improve I/O error handling for when dealing with FUSE filesystems by [@​jviotti](https://github.com/jviotti) in [#​651](sourcemeta/jsonschema#651) **Full Changelog**: <sourcemeta/jsonschema@v14.7.1...v14.7.2> ### [`v14.7.1`](https://github.com/sourcemeta/jsonschema/releases/tag/v14.7.1) [Compare Source](sourcemeta/jsonschema@v14.7.0...v14.7.1) #### What's Changed - Speed up `default`/`examples` linting on standalone schemas by [@​jviotti](https://github.com/jviotti) in [#​647](sourcemeta/jsonschema#647) - Publish the `musl` x86-64 binary to NPM by [@​jviotti](https://github.com/jviotti) in [#​649](sourcemeta/jsonschema#649) **Full Changelog**: <sourcemeta/jsonschema@v14.7.0...v14.7.1> ### [`v14.7.0`](https://github.com/sourcemeta/jsonschema/releases/tag/v14.7.0) [Compare Source](sourcemeta/jsonschema@v14.6.1...v14.7.0) #### What's Changed - Support formatting in the `lint` command using `--format` by [@​jviotti](https://github.com/jviotti) in [#​642](sourcemeta/jsonschema#642) - Print simple progress status when running `lint --fix` by [@​jviotti](https://github.com/jviotti) in [#​644](sourcemeta/jsonschema#644) - Support compiling a specific subschema in `compile` and `validate` by [@​jviotti](https://github.com/jviotti) in [#​646](sourcemeta/jsonschema#646) **Full Changelog**: <sourcemeta/jsonschema@v14.6.1...v14.7.0> ### [`v14.6.1`](https://github.com/sourcemeta/jsonschema/releases/tag/v14.6.1) [Compare Source](sourcemeta/jsonschema@v14.6.0...v14.6.1) #### What's Changed - Fix a lint `$dynamicAnchor` crash introduced in previous optimisations by [@​jviotti](https://github.com/jviotti) in [#​640](sourcemeta/jsonschema#640) **Full Changelog**: <sourcemeta/jsonschema@v14.6.0...v14.6.1> ### [`v14.6.0`](https://github.com/sourcemeta/jsonschema/releases/tag/v14.6.0) [Compare Source](sourcemeta/jsonschema@v14.5.0...v14.6.0) #### What's Changed - Remove NPM `postinstall` script as newer version of NPM no longer print the message by [@​Karan-Palan](https://github.com/Karan-Palan) in [#​633](sourcemeta/jsonschema#633) - Do not report non-fixable rules multiple times on `lint --fix` and add various new linter rules by [@​jviotti](https://github.com/jviotti) in [#​634](sourcemeta/jsonschema#634) - Optimise `lint --fix` to avoid re-analysing the schema in vain by [@​jviotti](https://github.com/jviotti) in [#​635](sourcemeta/jsonschema#635) - Mark orphaned locations in the `inspect` command as `orphan` by [@​jviotti](https://github.com/jviotti) in [#​636](sourcemeta/jsonschema#636) - Speed up linting of `default` and `examples` by [@​jviotti](https://github.com/jviotti) in [#​637](sourcemeta/jsonschema#637) - Speed up schema compilation, linting, and bundling by [@​jviotti](https://github.com/jviotti) in [#​638](sourcemeta/jsonschema#638) **Full Changelog**: <sourcemeta/jsonschema@v14.5.0...v14.6.0> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi45Ni4yIiwidXBkYXRlZEluVmVyIjoiNDIuOTkuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90IiwiYXV0b21hdGlvbjpib3QtYXV0aG9yZWQiLCJkZXBlbmRlbmN5LXR5cGU6Om1pbm9yIl19-->
See: #629
Signed-off-by: Juan Cruz Viotti jv@jviotti.com