updated to layer 143

[quetz]

Basic support for 139 that updates TL and what is necessary to get code compiled. At least client can now log in :)

[Lonami]

tdesktop's api.tl is in layer 143. Is there any reason to not go there directly?

[quetz]

Yeah, should probably update that to 143. And while we are at it...
Is there any specific reason why original .tl schemas do not work in grammers and require changes to them? (Maybe I should look into making original schemas work)
Is it ok to move from include!(concat!(env!("OUT_DIR"), "/generated.rs")); to putting generated.rs into src? Doing that will allow rust-analyzer's jump to definition work correctly.

[Lonami]

Is there any specific reason why original .tl schemas do not work in grammers and require changes to them?

If you mean things like:

int ? = Int;
long ? = Long;
double ? = Double;
string ? = String;

vector {t:Type} # [ t ] = Vector t;

int128 4*[ int ] = Int128;
int256 8*[ int ] = Int256;

They're "core" definitions which grammers must understand anyway, so I did not consider it necessary to complicate the parser for something that wouldn't be used. We can definitely improve the parse to accept those though, but the code generator will still ignore them. Feel free to do so in a separate PR. (I think Telegram Desktop also straight up ignores these.)

Is it ok to move from include!(concat!(env!("OUT_DIR"), "/generated.rs")); to putting generated.rs into src? Doing that will allow rust-analyzer's jump to definition work correctly.

I think I tried to do that but it didn't work. When I looked at the problem I'm pretty sure the solution / standard way was to use the out environment if generating code from a build.rs step (it's also pretty clean). I don't know if it's possible to put the file in the src/ when using a build.rs.

The "jump to definition" problem is definitely annoying, but if we used macros instead of code generation we still wouldn't have the definition,, so we have to stick with code generation and I don't know if that can put files in src/ during build.rs. If you manage let me know and we can talk more about the tradeoffs.

[quetz]

If you mean things like:

int ? = Int;
long ? = Long;
double ? = Double;
string ? = String;

vector {t:Type} # [ t ] = Vector t;

int128 4*[ int ] = Int128;
int256 8*[ int ] = Int256;

Not only those (and it should be safe to ignore those core types). Most errors come from the fact that TG authors (for some reason) use string type in crypto structs (like server_DH_params, while grammers use bytes. While bytes are better suited for the task sticking to original .tl files would make updating layers easier.

The "jump to definition" problem is definitely annoying, but if we used macros instead of code generation we still wouldn't have the definition,, so we have to stick with code generation and I don't know if that can put files in src/ during build.rs. If you manage let me know and we can talk more about the tradeoffs.

Just replacing Path::new(&env::var("OUT_DIR").unwrap()).join("generated.rs") with Path::new("src/generated.rs") worked for me (everything compiles, rust-analyzer works).

[Lonami]

Not only those (and it should be safe to ignore those core types). Most errors come from the fact that TG authors (for some reason) use string type in crypto structs (like server_DH_params, while grammers use bytes. While bytes are better suited for the task sticking to original .tl files would make updating layers easier.

As far as I remember, both bytes and string are synonyms in the TL (in fact the CRC calculation normalizes those to be the same name before calculating them). It's about semantics.

Because in Rust String must be valid UTF-8, and those "core" types will deal with non-UTF-8, we must either special-case them in the generator to force bytes or edit the TL. (For non-crucial types where string is used even though semantically it's bytes, it might be acceptable to have lossy conversion to String, although I don't remember any of those cases).

---

Just replacing Path::new(&env::var("OUT_DIR").unwrap()).join("generated.rs") with Path::new("src/generated.rs") worked for me (everything compiles, rust-analyzer works).

Alright, although then we might face Files generated by build script into src/ are compiled twice (it's an old issue, I have not checked if this is still the case), and both the suggestion:

[...] it's intended that build scripts generate files into $OUT_DIR and then they're included in the normal Rust source via:

include!(concat!(env!("OUT_DIR"), "/foo.rs"));]

...and the Cargo Book (emphasis mine):

Build scripts may save any output files or intermediate artifacts in the directory specified in the OUT_DIR environment variable. Scripts should not modify any files outside of that directory.

...seem to hint that we should not do that. Personally I agree that it's a bit dirty for a build script to write to the filesystem other than the controlled OUT_DIR location (even if it's still within the repository and we're not doing anything evil). I would love it if there was another way where the generated code could live in OUT_DIR but still be accessible by RA.

[quetz]

As far as I remember, both bytes and string are synonyms in the TL (in fact the CRC calculation normalizes those to be the same name before calculating them). It's about semantics.

Because in Rust String must be valid UTF-8, and those "core" types will deal with non-UTF-8, we must either special-case them in the generator to force bytes or edit the TL. (For non-crucial types where string is used even though semantically it's bytes, it might be acceptable to have lossy conversion to String, although I don't remember any of those cases).

Yeah, best solution here it to replace some string with bytes in reference .tl files. I'll create an issue in tdlib.

...seem to hint that we should not do that. Personally I agree that it's a bit dirty for a build script to write to the filesystem other than the controlled OUT_DIR location (even if it's still within the repository and we're not doing anything evil). I would love it if there was another way where the generated code could live in OUT_DIR but still be accessible by RA.

It is indeed somewhat dirty but I'll take that for a working RA. No problem though, I'll keep that change to my fork only.

Anyways, I've updated .tl to layer 143 (required some minor i32 -> i64 massaging but it works ok for me).

[Lonami]

Where's this TL from, tdesktop?

[quetz]

Where's this TL from, tdesktop?

https://github.com/tdlib/td
tdesktop should be using that lib too

[quetz]

What are drawbacks of putting generated.rs into src and committing to git? And manually re-generating it when schema is updated. This way RA will work fine and no double-compilation happens.

[Lonami]

Where's this TL from, tdesktop?

https://github.com/tdlib/td tdesktop should be using that lib too

I mean, I think I've always grabbed it from tdesktop (main reason being so the order of the definitions doesn't change much, making the diffs harder). The layer comment at the top made me ask this since tdesktop has it at the bottom.

What are drawbacks of putting generated.rs into src and committing to git? And manually re-generating it when schema is updated. This way RA will work fine and no double-compilation happens.

I don't feel all that comfortable writing outside of the designated OUT_DIR. Committing to git would also be a large unnecessary bloat. In any case this topic needs a separate issue or PR to talk about.

[Lonami]

Thanks.