Generated AscentProgram is a leaky abstraction

[regexident]

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.

[s-arash]

Hey @regexident, sure we can mark fields that (normally) shouldn't be touched as private, and it looks like a good idea. But keep in mind that simply making them private doesn't prevent the user from accessing them, unless the user defines ascent! {} in one module and uses it in another.

Feel free to open a PR for this.

[regexident]

But keep in mind that simply making them private doesn't prevent the user from accessing them, unless the user defines ascent! {} in one module and uses it in another.

This unfortunately has been necessary anyway, due to #12.

At least if you want to avoid getting spammed with warning's you can't actually resolve yourself, or if you're checking against cargo clippy on CI.

Feel free to open a PR for this.

Done: #58