Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify implementation of YAML witnesses #668

Merged
merged 6 commits into from
Aug 8, 2024

Conversation

schuessf
Copy link
Contributor

This PR attempts to simplify the implementation for YAML witnesses. The format has completely changed just before SV-COMP 2024 and thus the previous implementation for YAML witnesses was just an ad-hoc adaptation to the changed format.

  • Firstly the format for function contracts was changed, such that function contracts are also contained in the invariant_set (as in version 2.0), therefore the format for function contracts is downwards compatible and thus the format version was changed from 3.0 to 2.1
  • Previously each WitnessEntry had the methods toMap and toSetEntry for the string-conversion to format version 0.1 and 2.0 respectively. Instead of these methods, we have now two different writer-classes that decide which entries should be considered (function contracts should only be exported in version >= 2.1) and how the entries should be formatted in YAML.
  • The witness entry classes were initially made to represent the AST. However, since there are now multiple format versions, we use the classes just as container objects to store the necessary informations (e.g. no metadata). Therefore several classes were removed and we avoid parsing the unnecessary informations in YamlWitnessParser. Metadata are only needed for witness export, which are now produced using the introduced class MetadataProvider
  • Previously each witness entry had a set of labels as an unique identifier, to ensure in MainDispatcher that each entry is only instrumented once. This is however only specific to GraphML-witnesses. Therefore I added an interface IExtractedCorrectnessWitness as an abstraction for the extracted witness entries from GraphML and YAML witnesses and only the GraphML-implementation uses these labels internally. For YAML witnesses we can already ensure that each entry is only instrumented once during extraction.

After this PR is merged, #653 needs to be reworked.

Copy link
Member

@bahnwaerter bahnwaerter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your changes look great. Especially, the documentation comments increase code quality.
If omitting the access modifier is intentional, then I have nothing more to complain about.

location:
file_name: contract.c
file_hash: UNUSED
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅

trunk/source/WitnessPrinter/META-INF/MANIFEST.MF Outdated Show resolved Hide resolved
@schuessf schuessf force-pushed the wip/fs/yaml-witnnes-refactoring branch from 71b46da to a136772 Compare June 11, 2024 15:43
@schuessf schuessf force-pushed the wip/fs/yaml-witnnes-refactoring branch from a136772 to ce97273 Compare August 2, 2024 11:43
Copy link
Contributor

@maul-esel maul-esel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Thanks for all the work on improving this stuff :)

@schuessf schuessf force-pushed the wip/fs/yaml-witnnes-refactoring branch from ce97273 to 1a6cfd7 Compare August 7, 2024 14:45
- function_contracts are contained in the invariant_set
- the format for location_invariant and loop_invariant remains unchanged
- since the format is now downwards compatible, the version number changes from 3.0 to 2.1
- see https://gitlab.com/sosy-lab/benchmarking/sv-witnesses/-/tree/2.1-function-contracts-dev for the format for function contracts
We always generate the same witness independent from the format version.
Therefore WitnessSetEntry was removed and we have different writers instead.
The toMap method was removed from WitnessEntry, since it was specific to format version 0.1.
Only some common classes for the different format versions still provide a toMap method.
- only parse the entries from the YAML witness that are necessary for the validation
- produce the other entries only in YamlWitnessWriter without storing them in the witness object itself
- remove Invariant, Metadata, Producer, Task and produce nested maps directly in MetadataProvider and YamlWitnessWriter
This label is only required for GraphML. Instead there is now an interface IExtractedCorrectnessWitness with two implementations, one for GraphML and one for YAML, and only the GraphML implementation uses the label internally (using the internal class LabeledInvariant).
Therefore it is no more required to parse the UUID from the YAML witness.
This should be done by witness linter in SV-COMP
@schuessf schuessf force-pushed the wip/fs/yaml-witnnes-refactoring branch from 1a6cfd7 to dd23793 Compare August 7, 2024 15:02
@schuessf schuessf merged commit 19fd058 into dev Aug 8, 2024
3 of 4 checks passed
@schuessf schuessf deleted the wip/fs/yaml-witnnes-refactoring branch August 8, 2024 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants