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

Refactor repo into Cargo workspace with scaffolding for Rust-based queries #31

Merged
merged 21 commits into from
Sep 14, 2023

Conversation

willcrichton
Copy link
Collaborator

@willcrichton willcrichton commented Aug 31, 2023

What Changed?

The repository has become a Cargo workspace. The new crates directory contains:

  • dfpp: PDG generation, same as before
  • dfpp-explorer: the dfpp/explorer directory factored into a standalone crate
  • dfgraph: the core ProgramDescription type emitted by dfpp
  • dfcheck: the runtime for property checkers

I also added a props directory at the root which contains sample implementations of properties for Websubmit and Lemmy.

Why Does It Need To?

To separate out the different concerns, and to ensure that third-party property checkers don't need to include every dfpp transitive dependency.

Checklist

  • Above description has been filled out so that upon quash merge we have a
    good record of what changed.
  • New functions, methods, types are documented. Old documentation is updated
    if necessary
    • The new dfcheck and dfcheck-cli crates are not documented because their structure is highly tentative.
  • Documentation in Notion has been updated
    • Not sure where to update with this information?
  • Tests for new behaviors are provided
    • Need to add some tests that run the dfcheck-cli tool on the sample properties.
  • Refactor rustc serializers to just wholesale convert from rustc types to ProgramDescription before any serialization happens

Copy link
Collaborator

@JustusAdam JustusAdam left a comment

Choose a reason for hiding this comment

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

Nice work with the refactoring. Here are some thoughts I had on this code so far.

.gitignore Show resolved Hide resolved
crates/dfcheck/src/context.rs Show resolved Hide resolved
crates/dfcheck/src/context.rs Outdated Show resolved Hide resolved
crates/dfcheck/src/context.rs Outdated Show resolved Hide resolved
crates/dfcheck/src/flows_to.rs Show resolved Hide resolved
crates/dfgraph/Cargo.toml Outdated Show resolved Hide resolved
crates/dfgraph/src/lib.rs Outdated Show resolved Hide resolved
crates/dfpp-explorer/src/run.rs Outdated Show resolved Hide resolved
crates/dfpp/src/dbg.rs Outdated Show resolved Hide resolved
props/lemmy/build.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Copy link
Collaborator

@JustusAdam JustusAdam left a comment

Choose a reason for hiding this comment

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

I'm dot sure how much I agree with the df prefix. Since we're introducing new names we should make it consistent with the paper. I would suggest we call the graph crate spdg, and the checker policy-check. dfpp we can leave for now since people are already used to that.

.gitignore Show resolved Hide resolved
crates/dfcheck/src/lib.rs Show resolved Hide resolved
crates/dfgraph/Cargo.toml Show resolved Hide resolved
crates/dfgraph/src/lib.rs Outdated Show resolved Hide resolved
crates/dfgraph/src/rustc.rs Outdated Show resolved Hide resolved
crates/dfpp/build.rs Show resolved Hide resolved
crates/dfpp/src/ir/global_location.rs Outdated Show resolved Hide resolved
props/lemmy/build.rs Outdated Show resolved Hide resolved
props/lemmy/rust-toolchain.toml Outdated Show resolved Hide resolved
@JustusAdam
Copy link
Collaborator

I'm dot sure how much I agree with the df prefix. Since we're introducing new names we should make it consistent with the paper. I would suggest we call the graph crate spdg, and the checker policy-check. dfpp we can leave for now since people are already used to that.

Uhhh, maybe we could rename the dfpp create to spdgen? What do you think? cargo spdgen?

crates/dfgraph/src/global_location.rs Outdated Show resolved Hide resolved
crates/dfgraph/src/rustc_proxies.rs Show resolved Hide resolved
crates/dfgraph/src/rustc_proxies.rs Show resolved Hide resolved
crates/dfpp/src/utils/mod.rs Outdated Show resolved Hide resolved
crates/dfpp/src/frg.rs Show resolved Hide resolved
crates/dfgraph/src/global_location.rs Outdated Show resolved Hide resolved
crates/dfgraph/src/global_location.rs Show resolved Hide resolved
crates/dfgraph/src/global_location.rs Outdated Show resolved Hide resolved
crates/dfgraph/src/global_location.rs Show resolved Hide resolved
crates/dfgraph/src/global_location.rs Show resolved Hide resolved
crates/dfgraph/src/global_location.rs Outdated Show resolved Hide resolved
crates/dfgraph/src/global_location.rs Outdated Show resolved Hide resolved
crates/dfgraph/src/rustc_proxies.rs Show resolved Hide resolved
props/lemmy/src/lib.rs Outdated Show resolved Hide resolved
crates/dfpp/src/utils/mod.rs Outdated Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
Copy link
Collaborator

@JustusAdam JustusAdam left a comment

Choose a reason for hiding this comment

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

What about the build.rs for the properties? Are those actually necessary? And weren't you going to copy the build.rs to the explorer also?

@JustusAdam JustusAdam merged commit f4560a4 into main Sep 14, 2023
4 checks passed
@JustusAdam JustusAdam deleted the rust-queries branch September 20, 2023 15:33
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.

2 participants