On Thu, May 26, 2022 at 12:25 AM Nick Desaulniers <ndesaulniers@xxxxxxxxxx> wrote: > > Is there a reason to not just turn clippy on always? Might be nicer to > start off with good practices by default. :^) The idea crossed my mind too [1], but sadly Clippy disables some optimizations and in general is not intended to be used for normal compilation [2]. [1] https://github.com/rust-lang/rust-clippy/issues/8035 [2] https://github.com/rust-lang/rust-clippy/pull/8037 > Are there *.rmeta directories that we _don't_ want to remove via `make > mrproper`? I'm curious why *.rmeta isn't just part of MRPROPER_FILES? Putting them in `MRPROPER_FILES` would mean only those in a given directory are removed (e.g. the root one), but the idea here is to find them anywhere in the tree, since in the future we may have library crates in different parts of the tree. However, I am not sure I understand your first question in relation with the second one -- if we put them in `MRPROPER_FILES`, that would remove less, not more. > I don't think we need to repeat dir/* here again for rust. The > existing targets listed above (outside this hunk) make sense in both > contexts. The idea here is to document the differences (e.g. `RUSTFMT`) and that we may have other targets in the future that do not apply to C (e.g. MIR dump, if that happens to be useful). But maybe I can fit that in the normal place and make it still look good... I will give it a try. > Does rustc really use .i as a conventional suffix for macro expanded > sources? (The C compiler might use the -x flag to override the guess It is not a conventional suffix at all as far as I am aware. Maybe we should use a different one to aid editors and users anyway, e.g. `.rsi`, similar to `.mi` for Objective-C `.m` files. > it would make based on the file extension; I'm curious if rustc can > ingest .i files or will it warn?) The macro expanded output is not intended to be used as a compilable input, but as a debugging aid only. I can note this there. Having said that, `rustc` accepts the "preprocessed" output in the sense that it will just treat them as normal source files (no flag needed, and e.g. both `.i` and `.rsi` work); however, it may give new diagnostics or may require extra flags to enable some compiler features. >From a quick test, I managed to compile a "preprocessed" Rust kernel module with only command-line changes. But if we really wanted to support this, we would need to ask upstream Rust to actually support it. > Are these two kconfigs used anywhere? Not for the moment, but it could still be useful when getting `.config` reports (and we could add it to `LINUX_COMPILER` etc. in the future). > How does enabling or disabling debug info work for rustc? I may have > missed it, but I was surprised to see no additional flags getting > passed to rustc based on CONFIG_DEBUG info. `-Cdebuginfo=` is the one; by default there is no debug info and that option enables it (i.e. it is not just the level but the enablement too). (Note that in userspace, when compiling a common program, you will get some debug info coming from the standard library and other dependencies). > Ah, that explains the host rust build infra. Bravo! Hard coding the > target files was my major concern last I looked at the series. I'm > very happy to see it be generated properly from .config! > > I haven't actually reviewed this yet, but it makes me significantly > more confident in the series to see this approach added. Good job > whoever wrote this. Thanks! I thought Rust would be a nice option for writing hostprogs, though of course for the moment only programs that can assume Rust is there may use it. Note that non-x86 specific options need to be handled properly still (e.g. move things between the arch Makefiles and the hostprog as needed). I would still want to see compilers come to some kind of common format for this as we discussed other times, but... :) > Does `$(READELF) -p .comment foo.o` print anything about which > compiler was used? That seems less brittle IMO. Currently, `rustc` does not add a `.comment` section; but we could ask, I sent [3]. If debug info is enabled, another way is using the `DW_AT_language` attribute, like `pahole` will be doing for a new option we need [4]. However, we would still need a way for the other case. In any case, I am not sure something like `.comment` is less or more brittle -- compilers could in theory change it (e.g. not emitting it), while adding a symbol is something we control. So different kinds of brittleness. [3] https://github.com/rust-lang/rust/pull/97550 [4] https://git.kernel.org/pub/scm/devel/pahole/pahole.git/commit/?id=8ee363790b7437283c53090a85a9fec2f0b0fbc4 Cheers, Miguel
