From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 05E137493 for ; Sat, 2 Dec 2023 17:33:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="u22mTRE5" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A1978C433C8; Sat, 2 Dec 2023 17:33:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1701538430; bh=FyK3LZhs1D/WhL1Y2VzPXEFd97X8X85VHUVO563gFV4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=u22mTRE54ONIsymebB7GokeLCL6ccI08BSH2y6cigWDXrNjZXCiIpwq3IpF2WrRSh tWw6SF/BsoEqk4CA/oOLYTARsUcxvUp7oU/zwzGrme+uK2XPGRQBKxgGaY3hIr8Ajv Vt/vDT/lfqO3xmkBvXEAvS7gNodEtUVg7Fwr/iUImiXIBWaMDknavyAKtrOqZdRH9j pG79ZeaSGsgr+NGUGuhjYRMg5EHPTrYvKOXf5iWd+gG6YBIN1iqQajDDI1CjGNEhlA yR51erH+9lNDj7033e4zcLzuDHQ2MWjO/7tqwkd3UgR25n/7cj8VY6UZ18Zc3eOyzY nJk7tkPugcpxA== Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Sat, 02 Dec 2023 19:33:45 +0200 Message-Id: From: "Jarkko Sakkinen" To: "Benno Lossin" , "Miguel Ojeda" , "Alex Gaynor" , "Wedson Almeida Filho" , "Boqun Feng" , "Gary Guo" , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , "Andreas Hindborg" , "Alice Ryhl" , "Martin Rodriguez Reboredo" , "Asahi Lina" Cc: "Sumera Priyadarsini" , , Subject: Re: [PATCH 1/3] rust: macros: `parse_generics` add `decl_generics` X-Mailer: aerc 0.15.2 References: <20231125125024.1235933-1-benno.lossin@proton.me> <84da128f-17b3-4193-8389-176a8c961d64@proton.me> In-Reply-To: <84da128f-17b3-4193-8389-176a8c961d64@proton.me> On Sat Nov 25, 2023 at 5:39 PM EET, Benno Lossin wrote: > On 25.11.23 14:39, Jarkko Sakkinen wrote: > > Sorry, just went through my eyes, hope you don't mind I nitpick > > a bit. And maybe learn a bit in the process. > >=20 > > On Sat, 2023-11-25 at 12:50 +0000, Benno Lossin wrote: > >> When parsing generics of a type definition, default values can be > >> specified. This syntax is however only available on type definitions > >> and > >> not e.g. impl blocks. > >=20 > > Is "impl block" equivalent to a trait implementation? Maybe then just > > write in better English "trait implementation? Would be IMHO better > > to use commonly know terminology here. > > "impl block" refers to the syntactic item of Implementation [1]. It > might be a trait implementation, or an inherent implementation. To me > "impl block" is known terminology. > > [1]: https://doc.rust-lang.org/stable/reference/items/implementations.htm= l > > > Also for any commit, including any Rust commit. "When parsing" does > > not map to anything concrete. There always should be a concrete > > scenario where the parser its used. Especially since Rust is a new > > thing in the kernel, these commits should really have more in-depth > > information of the context. > > This commit is tagged `rust: macros:`, which means that it affects the > proc macros. So when I wrote "When parsing", I meant "When parsing Rust > code in proc macros". I will change this for v2. > > > I neither really grasped why trait implementations (if that is meant > > by "impl block") not having this support connects to the code change. > > Maybe just say that this patch adds the support and drop the whole > > story about traits. It is sort of unnecessary context. > > Rust does not syntactically support writing > > impl Foo { > } > > This is because it does not make sense. The syntax `=3D 0` only makes > sense on type definitions: > > struct Foo { > } > > Because then you can just write `Foo` and it will be the same type as > `Foo<0>`. Right. > > > Finally, why this change is needed? Any commit should have existential > > reason why it exists. So what will happen if "decl_generics" is not > > taken to the upstream kernel? How does it make life more difficult? > > You should be able to answer to this (in the commit message). > > Does this explain it?: > > In order to allow `#[pin_data]` on structs with default values for const > generic parameters, the `#[pin_data]` macro needs to parse them and have > access to the generics as they are written on the type definition. > This commit adds support for parsing them to the already present generics > parsing code in the macros crate. Yes. > > >> parameters. This patch also changes how `impl_generics` are made up, > >> as > >> these should be used with `impl<$impl_generics>`, they will omit the > >> default values. > >=20 > > What is decl_generics and what are the other _generics variables? > > This lacks explanation what sort of change is implemented and why. > > The terms `impl_generics` and `ty_generics` are taken from [2]. This > patch adds a third kind which also contains any default values of const > generic parameters. I named them `decl_generics`, because they only > appear on type declarations. > > [2]: https://docs.rs/syn/latest/syn/struct.Generics.html#method.split_for= _impl Thanks a lot of taking time for explaining all these concepts in a such a detail. Appreciate it! And I apologize for my a bit intrusive response. I really think that "more vocal and verbose" than "legacy C" patches would be a great policy for Rust specific patches. This would help audience who understand kernel but are not as in Rust to give more feedback on the patches. I mean tech is still the same whatever we used to implement the code that enables it. By doing that I see that all benefit and it opens doors for deeper Rust integration in the kernel. BR, Jarkko