Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new targets to run rustfmt for Rust projects #13932

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

bonzini
Copy link
Contributor

@bonzini bonzini commented Nov 20, 2024

<Leaving as draft until the base PR #13914 is merged>

@bonzini bonzini force-pushed the rustfmt branch 2 times, most recently from ed847b1 to ebb433c Compare November 20, 2024 19:33
@bonzini
Copy link
Contributor Author

bonzini commented Dec 6, 2024

Rebased on top of the updated #13914.

@bonzini bonzini force-pushed the rustfmt branch 3 times, most recently from d732c1f to 3ae4a45 Compare December 9, 2024 13:43
mesonbuild/scripts/clippy.py Dismissed Show dismissed Hide dismissed
from ..mesonlib import MachineChoice

if T.TYPE_CHECKING:
from ..compilers.rust import RustCompiler

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'RustCompiler' is not used.
There is no need to create and look up a dictionary when MachineChoice is
an enum, and there is no need to create a PerMachine object on every
__setitem__ of another PerMachine object.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
import mintro and its attendant module dependency tree just
so we can programmatically get filenames which are documented
as a stable API in https://mesonbuild.com/IDE-integration.html.

Suggested-by: Eli Schwartz <eschwartz93@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
It is useful to apply a limit to the number of processes even outside "meson test",
and specifically for clang tools.  In preparation for this, generalize
determine_worker_count() to accept a variable MESON_NUM_PROCESSES instead of
MESON_TESTTHREADS, and use it throughout instead of multiprocessing.cpu_count().

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Even though the "targets" introspection info already includes the
command line arguments used to invoke the compiler, this is not
enough to correlated with the "compilers" introspection info and
get extra information from there.

Together with the existing "language" key, adding a "machine" key
is enough to identify completely an entry in the compilers info.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Right now, the clang-tidy and clang-format targets use the program default
and do not let b_colorout decide whether to colorize output.

However, the wrappers that run the tool are going to be changed to buffer
output, and that would disable colorization unconditionally.  So pass
a --color option to the tools and use it when building the command line.

clang-format's -fcolor-diagnostics option simply does not work, and the
"right" (or at least working) option is --color which is undocumented.
--color is present all the way back to clang 10, but I digged into
clang-format's source code to figure out what's happening.  The problem
is that -fcolor-diagnostics is a complete no-operation; in fact it is
a bool that is initialized to true.  gdb shows:

  (gdb) p ShowColors
  $2 = {<llvm::cl::Option> = {
    ... <llvm::cl::opt_storage<bool, false, false>> = {Value = true,  ... }, ...}

on entry to clang-format's main, meaning that specifying the option on
the command line does nothing at all.

To see how clang-format determines whether to use colors you need to look
at enters SMDiagnostic::print, which simply does

  ColorMode Mode = ShowColors ? ColorMode::Auto : ColorMode::Disable;

showing once more that in fact the option cannot force-on the colors (
-fno-color-diagnostics instead works).  Continuing in SMDiagnostic::print,
this RAII constructor would write the escape sequence to the terminal:

  WithColor S(OS, raw_ostream::SAVEDCOLOR, true, false, Mode);

It ends up in WithColor::changeColor, which does

  if (colorsEnabled())
    OS.changeColor(Color, Bold, BG);

Digging further down, colorsEnabled() is where the Mode member is consulted:

  bool WithColor::colorsEnabled() {
    switch (Mode) {
      case ColorMode::Enable:
        return true;
      case ColorMode::Disable:
        return false;
      case ColorMode::Auto:
        return AutoDetectFunction(OS);
      }
      llvm_unreachable("All cases handled above.");
    }

and the "AutoDetectFunction" is

  static bool DefaultAutoDetectFunction(const raw_ostream &OS) {
    return *UseColor == cl::BOU_UNSET ? OS.has_colors()
                                      : *UseColor == cl::BOU_TRUE;
  }

UseColor is controlled by the "--color" option, so if that option was
unset you go to OS.has_colors() even in the presence of -fcolor-diagnostics.
This has been around for over 5 years in clang-format, and it was present
even earlier, so use it in meson as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Differentiate from the "run_tool_on_targets" function that will be introduced
in the next commit.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This improves the handling of keyboard interrupt, and also makes it easy to
buffer the output and not mix errors from different subprocesses.  This
is useful for clang-tidy and will be used by clippy as well.  In addition,
the new code supports MESON_NUM_PROCESSES.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Similar to the "ninja scan-build" target for C, add a clippy internal
tool that runs clippy-driver on all crates in the project.

The approach used is more efficient than with "ninja scan-build", and
does not require rerunning Meson in a separate build directory; it
uses the introspection data to find the compiler arguments for the
target and invokes clippy-driver with a slightly modified command
line.

This could actually be applied to scan-build as well, reusing the
run_tool_on_targets() function.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Add a target that builds all crates that could be extern to others,
and then reruns clippy.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
clippy-driver is not meant to be a general-purpose compiler front-end.
Since Meson can now provide natively the ability to invoke clippy,
raise a warning if someone uses it that way.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This is very similar to clippy, with different command line of course.
Also it can change files, so do not run it twice on the same file.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
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.

1 participant