Skip to content

Conversation

@carolynzech
Copy link
Contributor

@carolynzech carolynzech commented Mar 5, 2025

Follow up to #3874 implementing the features requested during review. Hopefully reviewing commit-by-commit makes the size manageable (most of the changes come from the additional metadata logging) but happy to split this into multiple PRs if not.

The list below maps each commit to the review comment on #3874 that inspired it.

Towards #3832

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

@carolynzech carolynzech requested a review from a team as a code owner March 5, 2025 20:10
@github-actions github-actions bot added the Z-EndToEndBenchCI Tag a PR to run benchmark CI label Mar 5, 2025
@carolynzech
Copy link
Contributor Author

cc @zhassan-aws and @tautschnig since you reviewed the last PR. (That doesn't mean you need to review this one too, but I thought I'd at least bring it to your attention in case you have opinions about how I addressed your comments).

@carolynzech carolynzech force-pushed the autoharness-cleanups branch from 5d41477 to 346655d Compare March 5, 2025 20:17
@carolynzech carolynzech force-pushed the autoharness-cleanups branch from 346655d to e160b65 Compare March 5, 2025 20:31
@carolynzech carolynzech force-pushed the autoharness-cleanups branch from e160b65 to 6997afb Compare March 5, 2025 20:40
Copy link
Member

@tautschnig tautschnig left a comment

Choose a reason for hiding this comment

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

Love the tables that are being generated!

@carolynzech carolynzech added this pull request to the merge queue Mar 6, 2025
Merged via the queue into model-checking:main with commit 68cb4ee Mar 6, 2025
24 of 25 checks passed
@carolynzech carolynzech deleted the autoharness-cleanups branch March 6, 2025 02:25
github-merge-queue bot pushed a commit that referenced this pull request Apr 17, 2025
## Summary
- fda814b: Make the autoharness filter
flags work on the standard library by moving them to
`kani_compiler_flags`, which ensures they're passed to all Kani compiler
invocations.
- c0430ec: Print the crate name in our
output tables.
- 093fc6b: **Breaking Change** to
rename `--include-function` and `--exclude-function` to mention
`pattern`s instead, which makes it clearer that they talk about
substrings of the total paths (e.g., modules). Also implement the
suggestion from
#3922 (comment)
so that the flags are no longer mutually exclusive.
- 9e35fca: Let the above flags filter
on crate names as well.
- 66444e4: Warn if an exclude flag
makes an include flag moot.

## Detail
Some more context on why f933799c54b09210cb267963ff1dc431c7a9385a allows
for both flags to be passed now: I realized as part of
#3984 how when we call `cargo
rustc` for a `cargo kani` invocation, we don't pass `--reachability` to
dependencies to avoid running harnesses in them. The problem is that we
can't do the same for our cargo command to build the standard library,
since that uses `cargo build`, which does not have the same ability to
pass flags only to the final compiler invocation and not the
dependencies. So we end up passing `--reachability=AllFns` to the
dependencies of the standard library as well and generate automatic
harnesses for them. If we can pass both filter flags, we can run
commands like `kani autoharness --std --include-pattern core
--exclude-pattern miniz_oxide`, which will include functions from the
`core` crate while excluding functions in the `miniz_oxide` that just
happen to have the word "core" in them.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Z-EndToEndBenchCI Tag a PR to run benchmark CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants