Skip to content

Impl trait contract#11

Merged
jct-tympanon merged 4 commits intomainfrom
issue-8
Mar 27, 2025
Merged

Impl trait contract#11
jct-tympanon merged 4 commits intomainfrom
issue-8

Conversation

@acdev123
Copy link
Copy Markdown
Contributor

A couple notes and limitations:

I disallow the usage of ! in the impl contract clause. This character is allowed per both the rust reference and inside of syn, but I can't for the life of me figure out what it's used for (the reference doesn't give any examples, maybe this is some Rust pattern I'm unfamiliar with). Let me know if you know and if we should validly support it.

Where clauses aren't allowed as there's no way I could find to validate them with static assertions.

I've also put this logic outside of the match clause for hoisting alias and use items since using an enum to manage multiple return types felt clunky, but if there's an idiomatic pattern for it you like I could move it in.

@acdev123 acdev123 requested a review from jct-tympanon March 25, 2025 17:08
@jct-tympanon
Copy link
Copy Markdown
Contributor

i'll take a look today.

i think there's active work around "negative impls", which use "!", in the language: https://rust-lang.zulipchat.com/#narrow/stream/144729-t-types/topic/Negative.20impls.20work/near/407068626

so maybe the "!" was allowed to break ground / future proof for that.

Copy link
Copy Markdown
Contributor

@jct-tympanon jct-tympanon left a comment

Choose a reason for hiding this comment

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

looks good! just one request (for doc), and one suggestion.

w.r.t. where clauses. it occurs to me that blanket implementations are something that can exist in the implementation rather than the contract. we make a static assertion that an impl exists with the contract; that impl may be achieved by direct declaration OR a blanket implementation in the loaded module. for that reason i think it might actually be inappropriate to make a "where clause" contract requirement. in any case, i suspect we won't miss them that much.

i am curious whether static assertions will work when the impl comes from a blanket implementation. i can't think of why they wouldn't. can you try that out in one of the examples? I think that would actually be a useful property to document / present via example, and a good test case.

Comment thread src/lib.rs
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you add an impl declaration to the code example in the doc? i think it's useful to demonstrate this syntax. i know we've got it in the examples but the rustdoc has a lot of value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added. It'll fail the doctest build, as I know we have a separate issue to make the doctest build work. As an aside, I know you assigned that task to yourself. If you're finding yourself busy with other tasks, you could reassign that to me and I could pick it up.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if you've got the time -- go for it.

Comment thread src/lib.rs
@acdev123
Copy link
Copy Markdown
Contributor Author

I did confirm that that static assertions does work with blanket implementations. The following code passed as an implementation. I can change the unsupported example to use a blanket implementation as an example.

impl<T> FilePathDescription<String> for T {
    fn description(&self) -> String {
        return "".to_string();
    }
}

@jct-tympanon
Copy link
Copy Markdown
Contributor

I did confirm that that static assertions does work with blanket implementations. The following code passed as an implementation. I can change the unsupported example to use a blanket implementation as an example.

impl<T> FilePathDescription<String> for T {
    fn description(&self) -> String {
        return "".to_string();
    }
}

I don't know if I was clear in my intent, but the specific idea was to do a blanket implementation over a "where" clause, where the impl contract was more specific.

#[platform_spi] 
example {
   impl SomeTrait for MyRequiredType {}
}

/// somewhere deep inside one of the platform modules
impl<T> SomeTrait for T where T: SomeBoundaryThatIncludesMyRequiredType {}

// the above should be sufficient to make static assertion happy?

@acdev123
Copy link
Copy Markdown
Contributor Author

Got it. Here's a deeper check including a where clause that passes:

impl ToString for UnsupportedImpl {
    fn to_string(&self) -> String {
        "This platform is unknown so we do not know how file paths are written.".to_string();
    }
}

//Example blanket implementation that fulfills contract
impl<T> FilePathDescription<String> for T
where T : ToString {
    fn description(&self) -> String {
        return self.to_string();
    }
}

If you don't implement ToString here static assertions will fail, so it behaves as you'd expect it should.

@jct-tympanon
Copy link
Copy Markdown
Contributor

Got it. Here's a deeper check including a where clause that passes:

impl ToString for UnsupportedImpl {
    fn to_string(&self) -> String {
        "This platform is unknown so we do not know how file paths are written.".to_string();
    }
}

//Example blanket implementation that fulfills contract
impl<T> FilePathDescription<String> for T
where T : ToString {
    fn description(&self) -> String {
        return self.to_string();
    }
}

If you don't implement ToString here static assertions will fail, so it behaves as you'd expect it should.

nice! i was thinking that 'where' clauses remain useful (and available) in implementation even if we don't support them in the contract specification. it may be the case that this is a useful limitation, since the true scope of a 'where' declaration at the contract level may be hard to reason about when you're considering many different, mutually exclusive platform implementations.

instead the contract just declares narrow invariants about explicit, known types. "you must define this named type; this named type must implement this named trait". how those invariants are actually achieved (via direct implementation or blanket implementation) can be different in each platform-specific module.

Comment thread src/lib.rs Outdated
/// pub use ErrorImpl as PlatformError;
///
/// /// Trait contract that specifies that each platform-specific PlatformService will implement SomeTrait
/// impl SomeTrait for PlatformService;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should be PlatformService {}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

if only it wasn't...

@jct-tympanon jct-tympanon merged commit 4513633 into main Mar 27, 2025
@jct-tympanon
Copy link
Copy Markdown
Contributor

i'll take a look today.

i think there's active work around "negative impls", which use "!", in the language: https://rust-lang.zulipchat.com/#narrow/stream/144729-t-types/topic/Negative.20impls.20work/near/407068626

so maybe the "!" was allowed to break ground / future proof for that.

I just remembered -- there are some established use cases for "!" like removing incorrectly computed autotraits like Send and Sync

/// explicitly declare that MyType cannot be safely referenced across threads, because for some reason the compiler thinks
/// that it can, and I know better. i'm probably doing something weird, which is not unheard of
impl !Sync for MyType {}

@acdev123
Copy link
Copy Markdown
Contributor Author

Interesting. Maybe this means that using "!" is a valid use case if it's more for convention. I'm not sure how that would play with the negative impls use case (those look like they use the same syntax with the "!" before the impl identifier, but the Rust reference shows "!" after the generic param declaration, so I'm not sure which syntax means what). I'd say we should forbid using it for now until it becomes more clear what "!" means in what contexts.

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