A proposal for a more comprehensive CLI property interface #38
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JustusAdam
changed the title
|
While this is making this PR multi-concern I did sketch your diagnostics approach @willcrichton. What do you think? |
|
Yeah this is basically what I was thinking. Re: your point about multi-property concurrency, this might also be a good time to implement multiple properties for one app. One way to do this is to give each property a unique name, and make the diagnostics a |
|
Maybe but I think we should merge this first and do that in a second step. |
|
I'd imagine an ergonomic way to do that is to have a |
6 tasks
JustusAdam
added a commit
that referenced
this pull request
Oct 2, 2023
## What Changed? Implements a suggestion from #38. Adds the ability to register simple context information, such as the currently executing policy or combinator with the `Context` that is emitted as part of the diagnostics. For instance the named policy `community-safety` in a hypothetical combinator `reaches` would output diagnostics such as: ``` [policy: community-safety] [reaches] Vacuous: no nodes matching starting conditions ``` This is achieved by transparently attaching diagnostic context. First off the `error` and `warning` convenience methods on `Context` still work. Both are now part of the `Diagnostics` trait, The trait is also implemented on `PolicyContext`, which is created by `Context::named_policy()` an transparently attaches the name of the policy to all diagnostic messages emitted from it. It `Deref`s to `Context` so all if its functionality is included. Both `Context` and `PolicyContext` can also create a `CombinatorContext` with the `named_combinator()` method that attaches a combinator name. `CombinatorContext` also `Deref`s to `Context`. ## Why Does It Need To? Right now every diagnostic message has to track where it occurs. By adding this way of accumulating contextual information we can, for instance, have a combinator like `always_happens_before` interrogate itself and dispatch generic diagnostics which are then enriched with contextual information, making them more specific and understandable again. ## Checklist - [x] Above description has been filled out so that upon quash merge we have a good record of what changed. - [x] New functions, methods, types are documented. Old documentation is updated if necessary - [x] A module level documentation section for how to use diagnostic context - [ ] Documentation in Notion has been updated - [ ] Tests for new behaviors are provided - [ ] New test suites (if any) ave been added to the CI tests (in `.github/workflows/rust.yml`) either as compiler test or integration test. *Or* justification for their omission from CI has been provided in this PR description.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What Changed?
Expands the
clifunction to a two-step workflow that includes running the dfpp graph generation and then reading that graph for property enforcement.Why Does It Need To?
Simplifies running properties to a single command as graph generation can be integrated and use some automatic configuration. At the same time leaves all control to the user still which can opt to run custom graph generation commands or do them completely manually.
Checklist
good record of what changed.
if necessary
.github/workflows/rust.yml) either as compiler test or integration test.Or justification for their omission from CI has been provided in this PR
description.