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

Generated AscentProgram is a leaky abstraction #48

Open
regexident opened this issue Oct 10, 2024 · 0 comments
Open

Generated AscentProgram is a leaky abstraction #48

regexident opened this issue Oct 10, 2024 · 0 comments

Comments

@regexident
Copy link
Contributor

Compiling an ascent! { … } program with relations edge and path

Program's full code snippet
use ascent::ascent;

ascent! {
   relation edge(i32, i32);
   relation path(i32, i32);
   
   path(x, y) <-- edge(x, y);
   path(x, z) <-- edge(x, y), path(y, z);
}

… with ascent v0.7.0 generates (and in the case of ascent! { … } exposes!) a program type with the following signature (as obtained by use of cargo expand):

pub struct AscentProgram {
    pub edge: ::std::vec::Vec<(i32, i32)>,
    pub __edge_ind_common: (),
    pub edge_indices_0_1: ascent::internal::RelFullIndexType<(i32, i32), ()>,
    pub edge_indices_1: ascent::rel::ToRelIndexType<(i32,), (i32,)>,
    pub edge_indices_none: ascent::rel::ToRelIndexType<(), (i32, i32)>,
    pub path: ::std::vec::Vec<(i32, i32)>,
    pub __path_ind_common: (),
    pub path_indices_0: ascent::rel::ToRelIndexType<(i32,), (i32,)>,
    pub path_indices_0_1: ascent::internal::RelFullIndexType<(i32, i32), ()>,
    scc_times: [std::time::Duration; 2usize],
    scc_iters: [usize; 2usize],
    pub update_time_nanos: std::sync::atomic::AtomicU64,
    pub update_indices_duration: std::time::Duration,
}

Other than edge and path none of these fields should be marked pub as all other fields are implementation details, turning AscentProgram into what effectively is a massively leaky abstraction and a violation of encapsulation.

None of those fields should actually have to be declared pub for AscentProgram to work, afaict.

As such rather than exposing most of its internal fields as pub only the fields corresponding to user-defined relations should be exposed via pub:

pub struct AscentProgram {
    pub edge: ::std::vec::Vec<(i32, i32)>,
    __edge_ind_common: (),
    edge_indices_0_1: ascent::internal::RelFullIndexType<(i32, i32), ()>,
    edge_indices_1: ascent::rel::ToRelIndexType<(i32,), (i32,)>,
    edge_indices_none: ascent::rel::ToRelIndexType<(), (i32, i32)>,
    pub path: ::std::vec::Vec<(i32, i32)>,
    __path_ind_common: (),
    path_indices_0: ascent::rel::ToRelIndexType<(i32,), (i32,)>,
    path_indices_0_1: ascent::internal::RelFullIndexType<(i32, i32), ()>,
    scc_times: [std::time::Duration; 2usize],
    scc_iters: [usize; 2usize],
    update_time_nanos: std::sync::atomic::AtomicU64,
    update_indices_duration: std::time::Duration,
}

This change would prevent the user from messing with the wrong parts of AscentProgram and significantly reduce the type's API surface (in the case of the fields of this particular dummy program the surface shrinks from 13 mostly irrelevant to just 2 actually relevant fields), while guiding the user to the appropriate fields much more clearly.

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

No branches or pull requests

1 participant