Introduce TypeList, always export Dependencies as well

[NyxCode]

@escritorio-gustavo Here's a proof of concept of the TypeList. Most notably, I've only added a new method TS::dependency_types() -> impl TypeList here, but I haven't touched the old TS::dependencies() -> Vec<Dependency> yet. I want to run a few tests first, and see if the output of dependency_types() is equivalent to the old dependencies()

[NyxCode]

Just to illustrate what this would enable:

fn export_dependencies<T: TS>() {
    struct Exporter;
    impl Visitor for Exporter {
        fn visit<T: TS>(&mut self) {
            T::export();
        }
    }

    T::dependency_types().for_each(&mut Exporter);
}

[escritorio-gustavo]

fn export_dependencies<T: TS>() {
   struct Exporter;
   impl Visitor for Exporter {
       fn visit<T: TS>(&mut self) {
           T::export();
       }
   }

   T::dependency_types().for_each(&mut Exporter);
}

This looks awesome! Absolutely love it! I'm heading out right now for the next 2 hours, but when I get back I'll pull this branch to get a better read

[NyxCode]

I am very suprised that that's the case, but it seems that dependencies() already gives the same output as dependency_types().
I was sure I fucked something up somewhere, but it seems like I didnt..

I ran that check over most of the test suite, and added a few contrived tests as well, but I cant manage to break it.

[escritorio-gustavo]

I am very suprised that that's the case, but it seems that dependencies() already gives the same output as dependency_types(). I was sure I fucked something up somewhere, but it seems like I didnt..

I ran that check over most of the test suite, and added a few contrived tests as well, but I cant manage to break it.

I too refuse to believe my code is ever working first time xD

[escritorio-gustavo]

It didn't fail CI! Is this somehow just thread safe? lol

[NyxCode]

It didn't fail CI! Is this somehow just thread safe? lol

Really curious about this as well. My theory is the following

• Thread 1 creates the file (File::create) and starts writing to it
• Thread 2 creates the file (File::create), but to the OS, it's a completely different inode.
• Thread 1 continues to write to the old file handle, not disturbing thread 2

It's hard to find a definite resource on this.

[escritorio-gustavo]

It's hard to find a definite resource on this.

Yeah, I'm trying to read on std::fs::write but I can't find anything in a similar situation, where the threads aren't coordinated by a single centralized place

[NyxCode]

An easy fix for the concurrent write issue would be to change export_type_to:

pub(crate) fn export_type_to<T: TS + ?Sized + 'static, P: AsRef<Path>>(
    path: P,
) -> Result<(), ExportError> {
    static FILE_LOCK: Mutex<()> = Mutex::new(());

    // ...

    let lock = FILE_LOCK.lock().unwrap();
    std::fs::write(path.as_ref(), buffer)?;
    drop(lock);

    Ok(())
}

That lock would be shared by every file export, so no two files would be written concurrently ever.

[NyxCode]

Though that issue is purely theoretical right now, so we could just do it & see if any1 encounters an issue - idk.

[escritorio-gustavo]

An easy fix for the concurrent write issue would be to change export_type_to:

pub(crate) fn export_type_to<T: TS + ?Sized + 'static, P: AsRef<Path>>(
    path: P,
) -> Result<(), ExportError> {
    static FILE_LOCK: Mutex<()> = Mutex::new(());

    // ...

    let lock = FILE_LOCK.lock().unwrap();
    std::fs::write(path.as_ref(), buffer)?;
    drop(lock);

    Ok(())
}

That lock would be shared by every file export, so no two files would be written concurrently ever.

Wait, this works? I always thought the mutex had to be shared between threads (hence the whole Arc<Mutex<T>> thing people like to talk about).

[escritorio-gustavo]

Or am I just missing something? As far as I can tell every thread is gonna run export_type_to on its own right? So each thread will have its own Mutex, wihch won't be shared

[NyxCode]

Or am I just missing something? As far as I can tell every thread is gonna run export_type_to on its own right? So each thread will have its own Mutex, wihch won't be shared

No, I don't think that's right. Because it's static, there is only one mutex somewhere in memory. Because that is the case, you could do

static SOMETHING: u32 = 125;
let reference: &'static u32 = &SOMETHING;

The reference can be static because the static lives forever.

Now, I had to look that up, but that the function export_type_to is generic doesn't change that:

A static item defined in a generic scope (for example in a blanket or default implementation) will result in exactly one static item being defined, as if the static definition was pulled out of the current scope into the module. There will not be one item per monomorphization.

[NyxCode]

You often see Arc<Mutex> because the Mutex does not live forever normally, when not in a static. So the Arc is there to reference-count and clean the mutex when it's no longer used. Besides using static, an other way to get rid of the Arc is by leaking it:

let x: &'static Mutex<()> = Box::leak(Box::new(Mutex::new(())));

Then, just like the static, you've got a &'static reference to it, which you can pass to every thread you want.

[escritorio-gustavo]

No, I don't think that's right. Because it's static, there is only one mutex somewhere in memory.

Oh now I get it! And this is definetly correct, I set up a small test to check with a function that sleeps and 20 tests calling it with and without the Mutex and also switching static with let

With static the tests really do wait for each other

[escritorio-gustavo]

In this test I also created a file with 20 paragraphs of lorem ipsum. Even without the Mutex I couldn't cause it to corrupt the file either

[escritorio-gustavo]

Okay, I've got the following:

#[cfg(test)]
fn do_it() {
    std::fs::write(PATH, CONTENT).unwrap();
}
#[test]
fn write_1() {
    for i in 0..10 {std::thread::spawn(|| do_it());}
}

#[test]
fn write_2() {
    for i in 0..10 {std::thread::spawn(|| do_it());}
}

#[test]
fn write_3() {
    for i in 0..10 {std::thread::spawn(|| do_it());}
}

// ...

#[test]
fn write_20() {
    for i in 0..10 {std::thread::spawn(|| do_it());}
}

const PATH: &str = "./foo.txt";
const CONTENT: &str = "VERY LONG STRING";

[escritorio-gustavo]

This is producing an empty file

[NyxCode]

Alright, awesome! That makes me feel less bad putting a Mutex in there! Though I think it's remarkable that it took 200 threads to get it to break.

[escritorio-gustavo]

Hold up, the way I did it there breaks, but when I join the threads, it's fine

#[test]
fn write_1() {
    let mut v = vec![];
    for i in 0..10 {
        v.push(std::thread::spawn(|| do_it()));
    }
    for i in v {
        i.join();
    }
}

[escritorio-gustavo]

Sitll even if std::fs::write happens to be thread safe when writing the same data to the same file, that's not explicitly documented to be the case and we probably shouldn't rely on it

[NyxCode]

From my side this is ready for merge! I'd like to aditionally expand the E2E tests a bit, but i'll do that in an other PR. So maybe you could take a last look, and if everything's alright merge this.

Again, huge thanks for exploring the problem space with me. It's actually fun maintaining OS stuff if there's some1 to do it with ❤️

[escritorio-gustavo]

It really is! I love to have someone to about this kinda stuff with!

[escritorio-gustavo]

I think it's ready for merging as well, though we have to remember to also merge #218 after this, as this is going to the e2e branch

[NyxCode]

Alright! A bit akward, but whatever ^^

[NyxCode]

Something which I completely forgot to pay attention to are self-referential structs! I've ran some tests (7.1.1 vs master), and this is how the behavious changed:

this: Box<Self>	on 7.1.1	on master
as-is	✅	✅
#[ts(flatten)]	❌ (runtime) stack overflow	❌ (compiler) "overflow evaluating the requirement"
#[ts(inline)]	❌ (runtime) stack overflow	❌ (compiler) "overflow evaluating the requirement"
#[ts(type = "any", flatten)]	❌ (macro) "... is not compatible ..."	❌ (macro) "... is not compatible ..."
#[ts(type = "any", inline)]	⚠️ works, but shouldn't	⚠️ works, but shouldn't

So what failed before with a stack overflow now fails at compile time - That's pretty cool, i think!

error[E0275]: overflow evaluating the requirement `<T as TS>::{opaque#0} == _`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0275`.
error: could not compile `ts-rs` (test "self_referential") due to 2 previous errors

---

Also, I realized that both 7.1.1 and master accept #[ts(type = "..", inline)] - As far as I can tell, inline never does anything when used together with type, right? So maybe we should emit an error for this, like we do for #[ts(type = "..", flatten)]

[NyxCode]

Found something interesting:

#[derive(TS)]
struct X<T> {
    a: T,
    b: Vec<X<(i32, T)>>
}

fails to compile with

thread 'rustc' has overflowed its stack
error: could not compile `ts-rs` (test "generics")

Tested both on 7.1.1 and master, so this is not a regression we introduces (recently). Might be interesting to find out why it's happening though. In a perfect world, it should compile and product

type X<T> = {
  a: T,
  b: Array<X<[number, T]>>,
} 

Will open an issue to keep track of this.

[escritorio-gustavo]

So what failed before with a stack overflow now fails at compile time - That's pretty cool, i think!

Agreed, if something is not gonna work it's far better to know about it at compile time

Also, I realized that both 7.1.1 and master accept #[ts(type = "..", inline)] - As far as I can tell, inline never does anything when used together with type, right? So maybe we should emit an error for this, like we do for #[ts(type = "..", flatten)]

Yeah, #[ts(type = "...", inline)] doesn't really make sense, we should emit an error for it