Skip to content

Lint: Use Ruff for all formatting in pre-commit #133123

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+162 −80
Open

Lint: Use Ruff for all formatting in pre-commit #133123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
65535ee
Use Ruff for all formatting in pre-commit
AA-Turner Apr 29, 2025
34c1850
Merge branch 'main' into ruff-format
AA-Turner May 1, 2025
ba8e4bf
Add Tools/jit/.ruff.toml
AA-Turner May 1, 2025
42ce014
PEP 8 (keep to 80 chars)
AA-Turner May 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
repos:
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.11.6
rev: v0.11.7
hooks:
- id: ruff
name: Run Ruff (lint) on Doc/
Expand All @@ -22,16 +22,13 @@ repos:
name: Run Ruff (format) on Doc/
args: [--check]
files: ^Doc/

- repo: https://github.com/psf/black-pre-commit-mirror
rev: 25.1.0
hooks:
- id: black
name: Run Black on Tools/build/check_warnings.py
- id: ruff-format
name: Run Ruff (format) on Tools/build/check_warnings.py
args: [--check, --config=Tools/build/.ruff.toml]
files: ^Tools/build/check_warnings.py
args: [--line-length=79]
- id: black
name: Run Black on Tools/jit/
- id: ruff-format
name: Run Ruff (format) on Tools/jit/
args: [--check, --config=Tools/jit/.ruff.toml]
files: ^Tools/jit/

- repo: https://github.com/pre-commit/pre-commit-hooks
Expand Down
1 change: 0 additions & 1 deletion Tools/build/check_warnings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ def get_unexpected_warnings(
"""
unexpected_warnings = {}
for file in files_with_warnings.keys():

rule = is_file_ignored(file, ignore_rules)

if rule:
Expand Down
5 changes: 5 additions & 0 deletions Tools/jit/.ruff.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
extend = "../../.ruff.toml" # Inherit the project-wide settings

[format]
preview = true
docstring-code-format = true
24 changes: 16 additions & 8 deletions Tools/jit/_llvm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
import typing

_LLVM_VERSION = 19
_LLVM_VERSION_PATTERN = re.compile(rf"version\s+{_LLVM_VERSION}\.\d+\.\d+\S*\s+")
_LLVM_VERSION_PATTERN = re.compile(
rf"version\s+{_LLVM_VERSION}\.\d+\.\d+\S*\s+"
)

_P = typing.ParamSpec("_P")
_R = typing.TypeVar("_R")
Expand All @@ -21,9 +23,7 @@ def _async_cache(f: _C[_P, _R]) -> _C[_P, _R]:
lock = asyncio.Lock()

@functools.wraps(f)
async def wrapper(
*args: _P.args, **kwargs: _P.kwargs # pylint: disable = no-member
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you mean to get rid of various pylint suppression comments in this PR? I assume it was there because it was useful to the JIT developers working on this code

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I'd meant to add them back after reformatting.

cc @brandtbucher to check that the comments are still useful (we don't run pylint in CI, currently)

) -> _R:
async def wrapper(*args: _P.args, **kwargs: _P.kwargs) -> _R:
async with lock:
if args not in cache:
cache[args] = await f(*args, **kwargs)
Expand All @@ -35,7 +35,9 @@ async def wrapper(
_CORES = asyncio.BoundedSemaphore(os.cpu_count() or 1)


async def _run(tool: str, args: typing.Iterable[str], echo: bool = False) -> str | None:
async def _run(
tool: str, args: typing.Iterable[str], echo: bool = False
) -> str | None:
command = [tool, *args]
async with _CORES:
if echo:
Expand All @@ -48,7 +50,9 @@ async def _run(tool: str, args: typing.Iterable[str], echo: bool = False) -> str
return None
out, _ = await process.communicate()
if process.returncode:
raise RuntimeError(f"{tool} exited with return code {process.returncode}")
raise RuntimeError(
f"{tool} exited with return code {process.returncode}"
)
return out.decode()


Expand All @@ -60,7 +64,9 @@ async def _check_tool_version(name: str, *, echo: bool = False) -> bool:

@_async_cache
async def _get_brew_llvm_prefix(*, echo: bool = False) -> str | None:
output = await _run("brew", ["--prefix", f"llvm@{_LLVM_VERSION}"], echo=echo)
output = await _run(
"brew", ["--prefix", f"llvm@{_LLVM_VERSION}"], echo=echo
)
return output and output.removesuffix("\n")


Expand Down Expand Up @@ -92,7 +98,9 @@ async def maybe_run(
return path and await _run(path, args, echo=echo)


async def run(tool: str, args: typing.Iterable[str], echo: bool = False) -> str:
async def run(
tool: str, args: typing.Iterable[str], echo: bool = False
) -> str:
"""Run an LLVM tool if it can be found. Otherwise, raise RuntimeError."""
output = await maybe_run(tool, args, echo=echo)
if output is None:
Expand Down
12 changes: 9 additions & 3 deletions Tools/jit/_schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ class COFFSection(typing.TypedDict):
class ELFSection(typing.TypedDict):
"""An ELF object file section."""

Flags: dict[typing.Literal["Flags"], list[dict[typing.Literal["Name"], str]]]
Flags: dict[
typing.Literal["Flags"], list[dict[typing.Literal["Name"], str]]
]
Index: int
Info: int
Relocations: list[dict[typing.Literal["Relocation"], ELFRelocation]]
Expand All @@ -110,11 +112,15 @@ class MachOSection(typing.TypedDict):
"""A Mach-O object file section."""

Address: int
Attributes: dict[typing.Literal["Flags"], list[dict[typing.Literal["Name"], str]]]
Attributes: dict[
typing.Literal["Flags"], list[dict[typing.Literal["Name"], str]]
]
Index: int
Name: dict[typing.Literal["Value"], str]
Relocations: typing.NotRequired[
list[dict[typing.Literal["Relocation"], MachORelocation]]
]
SectionData: typing.NotRequired[dict[typing.Literal["Bytes"], list[int]]]
Symbols: typing.NotRequired[list[dict[typing.Literal["Symbol"], _MachOSymbol]]]
Symbols: typing.NotRequired[
list[dict[typing.Literal["Symbol"], _MachOSymbol]]
]
80 changes: 51 additions & 29 deletions Tools/jit/_stencils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,18 @@ class Stencil:

body: bytearray = dataclasses.field(default_factory=bytearray, init=False)
holes: list[Hole] = dataclasses.field(default_factory=list, init=False)
disassembly: list[str] = dataclasses.field(default_factory=list, init=False)
disassembly: list[str] = dataclasses.field(
default_factory=list, init=False
)

def pad(self, alignment: int) -> None:
"""Pad the stencil to the given alignment."""
offset = len(self.body)
padding = -offset % alignment
if padding:
self.disassembly.append(f"{offset:x}: {' '.join(['00'] * padding)}")
self.disassembly.append(
f"{offset:x}: {' '.join(['00'] * padding)}"
)
self.body.extend([0] * padding)

def add_nops(self, nop: bytes, alignment: int) -> None:
Expand All @@ -230,35 +234,41 @@ def remove_jump(self) -> None:
"""Remove a zero-length continuation jump, if it exists."""
hole = max(self.holes, key=lambda hole: hole.offset)
match hole:
case Hole(
offset=offset,
kind="IMAGE_REL_AMD64_REL32",
value=HoleValue.GOT,
symbol="_JIT_CONTINUE",
addend=-4,
) as hole:
case (
Hole(
offset=offset,
kind="IMAGE_REL_AMD64_REL32",
value=HoleValue.GOT,
symbol="_JIT_CONTINUE",
addend=-4,
) as hole
):
# jmp qword ptr [rip]
jump = b"\x48\xff\x25\x00\x00\x00\x00"
offset -= 3
case Hole(
offset=offset,
kind="IMAGE_REL_I386_REL32" | "R_X86_64_PLT32" | "X86_64_RELOC_BRANCH",
value=HoleValue.CONTINUE,
symbol=None,
addend=addend,
) as hole if (
_signed(addend) == -4
):
case (
Hole(
offset=offset,
kind="IMAGE_REL_I386_REL32"
| "R_X86_64_PLT32"
| "X86_64_RELOC_BRANCH",
value=HoleValue.CONTINUE,
symbol=None,
addend=addend,
) as hole
) if _signed(addend) == -4:
# jmp 5
jump = b"\xe9\x00\x00\x00\x00"
offset -= 1
case Hole(
offset=offset,
kind="R_AARCH64_JUMP26",
value=HoleValue.CONTINUE,
symbol=None,
addend=0,
) as hole:
case (
Hole(
offset=offset,
kind="R_AARCH64_JUMP26",
value=HoleValue.CONTINUE,
symbol=None,
addend=0,
) as hole
):
# b #4
jump = b"\x00\x00\x00\x14"
case _:
Expand All @@ -285,13 +295,21 @@ class StencilGroup:
_trampolines: set[int] = dataclasses.field(default_factory=set, init=False)

def process_relocations(
self, known_symbols: dict[str, int], *, alignment: int = 1, nop: bytes = b""
self,
known_symbols: dict[str, int],
*,
alignment: int = 1,
nop: bytes = b"",
) -> None:
"""Fix up all GOT and internal relocations for this stencil group."""
for hole in self.code.holes.copy():
if (
hole.kind
in {"R_AARCH64_CALL26", "R_AARCH64_JUMP26", "ARM64_RELOC_BRANCH26"}
in {
"R_AARCH64_CALL26",
"R_AARCH64_JUMP26",
"ARM64_RELOC_BRANCH26",
}
and hole.value is HoleValue.ZERO
and hole.symbol not in self.symbols
):
Expand All @@ -314,7 +332,9 @@ def process_relocations(
if hole.value is HoleValue.GOT:
assert hole.symbol is not None
hole.value = HoleValue.DATA
hole.addend += self._global_offset_table_lookup(hole.symbol)
hole.addend += self._global_offset_table_lookup(
hole.symbol
)
hole.symbol = None
elif hole.symbol in self.symbols:
hole.value, addend = self.symbols[hole.symbol]
Expand All @@ -332,7 +352,9 @@ def process_relocations(
self.data.holes.sort(key=lambda hole: hole.offset)

def _global_offset_table_lookup(self, symbol: str) -> int:
return len(self.data.body) + self._got.setdefault(symbol, 8 * len(self._got))
return len(self.data.body) + self._got.setdefault(
symbol, 8 * len(self._got)
)

def _emit_global_offset_table(self) -> None:
got = len(self.data.body)
Expand Down
Loading
Loading