lint ImproperCTypes: overhaul (take 2 of "better handling of indirections") #134697

niacdoial · 2024-12-23T17:59:31Z

This PR started as a try to re-add the changes of #131669 (reverted in #134064 after one (1) nightly)

It has since evolved in an overhaul of the ImproperCTypes lints, splitting the two lints into four:

one for "imported" items in extern "ABI" blocks
one for "exported" extern "ABI" functions
one for "exported" #[no_mangle]/1[export_name=_] static variables
one for extern "ABI" callbacks (ty::FnPtr arguments/return types/ADT fields)

It also allows for "detailed" lint messages, with successive notes that amount to "this is unsafe because of that, which is unsafe because of that, which is unsafe for this reason", and possible help messages at every step.

Hopefully the overall architecture of the lint is something less special-case-y, with better-separated abstractions.

the comment containing the most recent summary of the decisions and considerations is #134697 (comment)

oh, also, since this was already fixing it for the wrong reasons, I decided to add something that fixes it for the right ones:
Fixes #130310

Fixes #132699 (though not necessarily for the right reasons?)

(leaving this in because it was in the original version of this description:)
r? workingjubilee (because you reviewed the first attempt)

niacdoial · 2024-12-23T18:03:42Z

(
hi Jubilee, I'm back at this again!
I know this is not the best time of year to add PRs, so I'm fine with postponing this if you don't feel like tackling it these upcoming weeks.
In any case, have some nice end-of-year festivities, if you celebrate any!
)

niacdoial · 2024-12-23T18:08:24Z

ah, and before I forget: a small part of the new test file is commented out because it hits ICE #134587, but there should be more than decent coverage anyway

workingjubilee · 2024-12-24T09:16:24Z

unfortunately the lint needs to be gutted and rewritten.

workingjubilee · 2024-12-24T09:18:44Z

Also while I was possibly having a mild case of get-there-itis and thus mostly tried to just make sure things were coherent, I would prefer all new code for the lint be in compiler/rustc_lint/src/types/improper_ctypes.rs.

workingjubilee · 2025-01-04T18:24:23Z

The version cut happened so there will be less time pressure now.

niacdoial · 2025-01-05T00:29:14Z

I have a first version for compiler/rustc_lint/src/types/improper_ctypes.rs if you want.
it's less of a from-the-ground-up rewrite as it is scrapping the original for parts, if the analogy makes sense.

you probably have things to say about its architecture, even if the whole thing still have a bunch of TODO comments
the progress so far looks like this:

completely separate the type-checking and reporting systems
(part of the way there) remove some of the special cases and integrate them to the "main logic"
- check_for_opaque_types is still a "special case" part of the checking logic
- Cstr and Cstring are also somewhat special-cased because the advice for them depends on the type around them, if any
- the unit type is handled in multiple places, see if this can be fixed
(almost complete) compile, pass existing tests
- only failed tests are for Cstring, due to different error messages
- one unrelated test had to have a second "#[allow(improper_ctypes)]" added, but it makes more sense for it to need that anyway
better separation of the different checks in different visit_* methods of ImproperCTypesVisitor
better tracking of how the currently-checked type is used (static, function argument, function return's inner type, etc...)
- raises questions about the separation of improper_ctypes and improper_ctypes_definitions versus declared/defined functions, especially when FnPtr:s are involved
allow single argument check to emit multiple errors (for fnptr:s, structures with multiple FFI-unsafe fields, etc)
review what is considered FFI-safe or not (once everything else is complete)

If you want to take a look in this state, should I just commit it here? (possibly put the PR in draft mode while I'm at it?)
or send you the files in a different way?

bors · 2025-01-15T12:53:21Z

☔ The latest upstream changes (presumably #135525) made this pull request unmergeable. Please resolve the merge conflicts.

niacdoial · 2025-01-18T20:20:25Z

aaaand I think I have something that's "first draft" material! (should I put this pull in the "draft" state?)
There's still a bunch of ~~TODOs~~ (...well, they were changed to FIXMEs to be pushed here) for questions I didn't manage to answer (and a bunch of failing tests because I don't know if the error should be here), but yeah.

here's a list of some of my remaining questions and concerns:

visit_numeric seems too x86_64-specific
should we revisit the distinction between ImproperCTypes and ImproperCTypesDefinitions?
- part 1: the output ("external fn" or vs "external block" vs other possibilities)
- part 2: handling opaque types (there's a high correlation between ImproperCTypesDefinitions and places where we allow FFI-opaque types to be fully specified. do we want this correlation to be 1?)
more on FFI-opaque types: how do we handle that in the context of the "context switch" between functions and possible FnPtr arguments? The answer that seems correct currently prevents a stage1 compiler from being built
- should we introduce a std::ffi::FfiOpaquePtr type? (which would be a *const c_void and some phantomdata, on first approximation)
for indirections whose values may be supplied by non-rust code: do we only allow pointers (and Optionstd::ptr::NonNull), or do we also allow Option<&T> and Option<Box<T>>?
not sure if the new error messages are intelligible in all cases (especially if there's a type param like Self or <Self as ::std::ops::Add<Self>>::Output that gets resolved in the error message).
it feels like the current handling of CStr/Cstring and Option-like enums uses special casing, since those are tested for in multiple places.
if we deny references and boxes in defined functions, what of &self in methods? We don't allow *const Self, last I checked.

bors · 2025-01-23T18:22:31Z

☔ The latest upstream changes (presumably #135921) made this pull request unmergeable. Please resolve the merge conflicts.

workingjubilee · 2025-01-31T05:14:25Z

hmm.

workingjubilee · 2025-01-31T05:15:35Z

aaaand I think I have something that's "first draft" material! (should I put this pull in the "draft" state?)

Yes, it's a good marker for "I don't want this merged yet, even if it looks done".

workingjubilee · 2025-02-20T18:35:04Z

visit_numeric seems too x86_64-specific

It probably is.

should we revisit the distinction between ImproperCTypes and ImproperCTypesDefinitions?

Yes, but in particular, not to just repartition them between: I think breaking them into as many conceptually-smaller lints as possible is good, as long as each one is a distinct idea (no splitting just for the sake of splitting!).

more on FFI-opaque types: how do we handle that in the context of the "context switch" between functions and possible FnPtr arguments? The answer that seems correct currently prevents a stage1 compiler from being built

I'm not sure what you mean?

for indirections whose values may be supplied by non-rust code: do we only allow pointers (and Option<std::ptr::NonNull>), or do we also allow Option<&T> and Option<Box<T>>?

We must allow Rust code to declare a pointer in a C signature to be Option<&T> or a number of things about our FFI story fall apart.

it feels like the current handling of CStr/CString and Option-like enums uses special casing, since those are tested for in multiple places.

Yes, probably.

workingjubilee

some initial nits on a first pass

compiler/rustc_lint/src/types/improper_ctypes.rs

workingjubilee · 2025-02-20T21:45:48Z

compiler/rustc_lint/src/types/improper_ctypes.rs

+                        // but for some reason one can just go and write function *pointers* like that:
+                        // `type Foo = extern "C" fn(::std::ffi::CStr);`


Because unsized function parameters are something we may want to support.

The code may not be well-formed: as you may have noticed at some point, you get warnings even if you get errors (usually), and this is because we lint even on "bad" code. This is because rustc didn't use to, once upon a time, and it was a bad debugging experience.

compiler/rustc_lint/src/types/improper_ctypes.rs

niacdoial · 2025-02-27T22:12:58Z

alright, sorry for taking a while!

I'm currently planning what changes I'll do in terms of splitting the lint(s)
my current idea is to separate based on the nature of the thing being (presumably) propped up against a FFI boundary

improper_ctypes: what's definitely an interface to an outside library (extern statics, extern function declarations)
improper_ctypes_fn_definitions: functions written in rust intended to be exported
improper_ctypes_callbacks: FnPtr arguments, no matter in what function they are being used
improper_ctypes_ty_definitions: repr(C) structs/enums(/unions)?

more on FFI-opaque types: how do we handle that in the context of the "context switch" between functions and possible FnPtr arguments? The answer that seems correct currently prevents a stage1 compiler from being built

I'm not sure what you mean?

well, this is more or less answered in what I said before that, but my question was about how to deal with "switching" from checking arguments for, say, a function definition, to checking the arguments of a FnPtr argument?

should the nature of the lint change?
(temptative answer: yes)
how should FFI-Safe-pointers-to-FFI-Unsafe-pointees work in FnPtr arguments? Should it be the rules for extern fn declarations? (throw the lint because one should use *const c_void, an extern type declaration, etc...) or the rules for extern fn definitions? (allow that, the function's body needs the full type even if it's opaque to the other side of the FFI boundary)
(temptative answer: it should be the former, but parts of the rustc codebase doesn't follow this rule, so I can't get a stage1 compiler if I make that the rule)

// you would think that int-range pattern types that exclude 0 would have Option layout optimisation
// they don't (see tests/ui/type/pattern_types/range_patterns.stderr)
// so there's no need to allow Option<pattern_type!(u32 in 1..)>.

oh, I should fix that probably

I... maybe? I can't for the life of me find the link to that again but I think I saw a discussion about that and type covariance/contravariance,
where i32 is 1.. is a subtype of i32 (well that was under consideration), meaning fn(Option<i32>) is a type of fn(Option<i32 is 1..>) and it might have impacts on whether there should be an optimisation because of transmutation?

Though you'll definitely know more than me on all the moving parts.
Especially assuming you might have looked at this more in the past week.

As for the rest of your advice, I already took all this in!
thanks for shedding light on my code, one nit at a time!

rustbot · 2025-08-29T21:58:22Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

tgross35

Thank you for the history cleanup; looking at the first few commits, things seem to make a lot more sense. I did a quick breeze through the first ~8 patches and left some comments.

One general request to help reviewability: could you try to add some more detail to the commit messages? To say what the motivation is and what the changes are, or if it's just a refactoring with no visible changes. E.g. "change X linting" and the "change handling of X" commits, what do they actually change - just the diagnostic wording? catch something new? fix a false positive? fix something broken? move to a different group? refactoring? etc. Basically a mini PR description :)

View changes since this review

tgross35 · 2025-08-29T23:15:54Z

tests/ui/lint/improper_ctypes/lint-113436-1.rs

Nit for commit 1 "move code and tests into proper directories": the directory should be improper-ctypes (underscores usually turn into hyphens for prose, and we use that form for tests)

fixed this near the end of the commit chain
(for some reason jj-vcs doesn't know how to propagate "rename this directory" across a commit's descendants, so fixing every commit downstream looked to be too tedious)

compiler/rustc_lint/src/foreign_modules.rs

tgross35 · 2025-08-29T23:28:48Z

compiler/rustc_lint/src/types/improper_ctypes.rs

+#[allow(non_snake_case)]
+mod CTypesVisitorStateFlags {


From commit 4 "redo state tracking"

bitflags is in the graph already so you may as well use it here. Since it's a workspace dep, bitflags.workspace = true in compiler/rustc_lint/Cargo.toml should work. I don't think the separate enum and flags is needed in that case, you'll e.g .be able to impl CTypesVisitorState { const ARG_TY_IN_DEFINITION: Self = Self::FUNC | Self::DEFINED }.

If this is kept as a module, it would be preferable to keep the snake case convention (use CTypesVisitorStateFlags::* looks unusual). Or just put these in an impl CTypesVisitorState block.

Also if you're looking to shorten the name up a bit, VisitorState is probably sufficient for the flag type since we're already in the ctypes module.

Did this, the only issue is that I had to work around the bitwise_or operator (for this type) not being available in const contexts.

tgross35 · 2025-08-29T23:41:44Z

compiler/rustc_lint/src/types/improper_ctypes.rs

+    /// whether the type is used (directly or not) in a defined function
+    /// in other words, whether or not we allow non-FFI-safe types behind a C pointer,
+    /// to be treated as an opaque type on the other side of the FFI boundary
+    fn is_in_defined_function(self) -> bool {
+        use CTypesVisitorStateFlags::*;
+        ((self as u8) & DEFINED) != 0 && self.is_in_function()
+    }
+
+    /// whether the type is used (directly or not) in a function pointer type
+    /// here, we also allow non-FFI-safe types behind a C pointer,
+    /// to be treated as an opaque type on the other side of the FFI boundary
+    fn is_in_fnptr(self) -> bool {


Nit: doc comments are prose, with capital letters and periods at the end of sentences (applies quite a few places in this commit and across others). Obviously this isn't critical.

Not sure I got all of them, but I did change that

compiler/rustc_lint/src/types/improper_ctypes.rs

tgross35 · 2025-08-29T23:50:24Z

tests/ui/lint/improper_ctypes/allow_improper_ctypes.rs

Nit on one of the last commits "add tests": it would be awesome to move this commit to the beginning, basically with a bunch of FIXMEs where the behavior is wrong, then update the test output after each commit so we can see how the behavior changes. That can be tricky though, so don't worry too much if this isn't easily doable.

I think you did do this for a handful of the other tests.

the three tests there don't really add anything interesting to see the evolution of, IMHO.

one is to see where #[allow(improper_c*)] is effective (it's not that great but usable)

one is to enshrine that other issue I fixed without meaning to

and the last is a more extensive version of the test put in place after Revert #131669 due to ICEs #134064, making sure all ty_kinds are covered behind indirections.

tests/ui/lint/improper_ctypes/allow_improper_ctypes.rs

compiler/rustc_lint/src/types/improper_ctypes.rs

Mainly, we realise that the non-null assumption on a Box<_> argument does not depend on what side of the FFI boundary the function is on. And anyway, this is not the way to deal with this assumption being maybe violated.

no visible changes to rust users, just making the inner architecture of the ImproperCTypes lints more sensible, with a clean separation between the struct (now singular) that interacts with the linting system and the struct (now singular) that visits the types to check FFI-safety

No changes should be visible by rustc users This is just some architecture changes to the type checking to facilitate FFI-safety decisions that depend on how the type is used (the change here is not complete, there are still bits of "legacy" state passing for this, but since this is a retconned commit, I can tell you those bits will disappear before the end of the commit chain) (there is at least one bit where the decision making code is weird, but that this is because we do not want to change the lint's behaviour this early in the chain)

Another interal change that shouldn't impact rustc users. The goal is to break apart the gigantic visit_type function into more managable and easily-editable bits that focus on specific parts of FFI safety.

Another change that only impacts rustc developers: Added the necessary changes so that lints are able to specify in detail "A in unsafe because of its B field, which in turn is unsafe because of C, etc", and possibly specify multiple help messages (multiple ways to reach FFI-safety)

Simple change to stop irregular recursive types from causing infinitely-deep recursion in type checking.

First retconned commit in this change to impact rustc users: The improper_ctypes_definitions has been split into improper_c_fn_definitions and improper_c_callbacks, with the former lint name being turned into a lint group, so that users aren't forced to immediately change their code. Deprecating this old name will be left as an exercise to whichever team is in charge of breaking changes. Another lint group has been created to deal with all `improper_c*` lints at once.

another user-visible change: change the messaging and help around CStr/CString lints

- Uniformise how indirections (references, Boxes, raw pointers) are handled. (more specific indirection types with specific guarantees are not handled yet) - Indirections that are compiled to a "thick pointer" (indirections to slices, dyn objects, *not* foreign !Sized types) have better messaging around them. - Now, the pointee of a FFI-safe indirection is always considered safe. This might be a regression, if we consider that an extern function's API should describe how the function can be used by the non-defining side of the FFI boundary. However, enforcing this everywhere would force the user to perform an unreasonable amount of typecasts to/from opaque pointers. There is something better to do here, but it will be left to another PR.

Notably, those FnPtrs are treated as "the item impacted by the error", instead of the functions/structs making use of them.

A major change to the content of linting messages, but not where they occur. Now, the "uses type `_`" part of the message mentions the type directly visible where the error occurs, and the nested note/help messages trace the link to the actual source of the FFI-unsafety

A change in how type checking goes through structs/enums/unions, mostly to be able to yield multiple lints if multiple fields are unsafe

correctly handle !Sized arrays at the tail-end of structs and a cosmetic change to the array/slice-related lints,

Add some logic to the type checking to refuse uninhabited types as arguments, and treat uninhabited variants of an enum as FFI-safe if at least one variant is inhabited.

Properly treat the fact that a pattern can create assumptions that are used by Option-like enums to be smaller, making those enums FFI-safe without `[repr(C)]`.

Put the handling of opaque aliases in the actual `visit___` methods instead of awkwardly pre-checking for them

Add new areas that are checked by ImproperCTypes lints: Function declarations(*) and definitions in traits and impls *) from the perspective of an FFI boundary, those are actually definitions

Added the missing case for FFI-exposed pieces of code: static variables with the `no_mangle` or `export_name` annotations. This adds a new lint, which is managed by the rest of the ImproperCTypes architecture.

`[repr(C,packed)]` structs shouldn't be considered FFI-safe

- ensure proper coverage of as many edge cases in the type checking as possible - test for an issue that was fixed by this commit chain - understand where `[allow(improper_c*)]` needs to be to take effect (current behaviour not ideal, one should be able to flag a struct definitition as safe anyway)

smooth things out to avoid conflicts with rust-lang/compiler-builtins#1006 which has at time of writing not made it into rust-lang/rust's main branch

Rustc is trying to shift away from test names that are just issue numbers, so we add this as part of its effort, since this commit chain is moving and rewriting these files anyway. The directory containing all tests is also renamed.

rustbot assigned workingjubilee Dec 23, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 23, 2024

workingjubilee added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 4, 2025

niacdoial force-pushed the linting-ptrdyn-ffi branch from 5764b36 to 6c19cff Compare January 18, 2025 20:20

This comment has been minimized.

Sign in to view

niacdoial force-pushed the linting-ptrdyn-ffi branch from 6c19cff to 13720e7 Compare January 28, 2025 23:34

This comment has been minimized.

Sign in to view

workingjubilee added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 31, 2025

workingjubilee self-requested a review January 31, 2025 05:14

workingjubilee marked this pull request as draft January 31, 2025 05:15

workingjubilee reviewed Feb 20, 2025

View reviewed changes

workingjubilee added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 20, 2025

workingjubilee reviewed Feb 21, 2025

View reviewed changes

ImproperCTypes: move code and tests into proper directories

cc1cb49

niacdoial force-pushed the linting-ptrdyn-ffi branch from 0d80e12 to bdf51de Compare August 29, 2025 21:58

niacdoial force-pushed the linting-ptrdyn-ffi branch from bdf51de to f614086 Compare August 29, 2025 22:01

This comment has been minimized.

Sign in to view

niacdoial force-pushed the linting-ptrdyn-ffi branch from f614086 to 22b00d0 Compare August 29, 2025 22:57

tgross35 reviewed Aug 30, 2025

View reviewed changes

niacdoial added 22 commits August 30, 2025 21:50

ImproperCTypes: more pre-emptive cleanup

5ac6788

Mainly, we realise that the non-null assumption on a Box<_> argument does not depend on what side of the FFI boundary the function is on. And anyway, this is not the way to deal with this assumption being maybe violated.

ImproperCTypes: split type visiting into subfunctions

585945c

Another interal change that shouldn't impact rustc users. The goal is to break apart the gigantic visit_type function into more managable and easily-editable bits that focus on specific parts of FFI safety.

ImproperCTypes: add recursion limit

1470565

Simple change to stop irregular recursive types from causing infinitely-deep recursion in type checking.

ImproperCTypes: change cstr linting

b80c5ae

another user-visible change: change the messaging and help around CStr/CString lints

ImproperCTypes: change handling of FnPtrs

3d4ce20

Notably, those FnPtrs are treated as "the item impacted by the error", instead of the functions/structs making use of them.

ImproperCTypes: change handling of ADTs

fe31372

A change in how type checking goes through structs/enums/unions, mostly to be able to yield multiple lints if multiple fields are unsafe

ImproperCTypes: change handling of slices

7d195f6

correctly handle !Sized arrays at the tail-end of structs and a cosmetic change to the array/slice-related lints,

ImproperCTypes: handle uninhabited types

58d58ea

Add some logic to the type checking to refuse uninhabited types as arguments, and treat uninhabited variants of an enum as FFI-safe if at least one variant is inhabited.

ImproperCTypes: handle the Option<pattern> case

14e4868

Properly treat the fact that a pattern can create assumptions that are used by Option-like enums to be smaller, making those enums FFI-safe without `[repr(C)]`.

ImproperCTypes: refactor handling opaque aliases

2f27c6e

Put the handling of opaque aliases in the actual `visit___` methods instead of awkwardly pre-checking for them

ImproperCTypes: also check in traits

fb70d73

Add new areas that are checked by ImproperCTypes lints: Function declarations(*) and definitions in traits and impls *) from the perspective of an FFI boundary, those are actually definitions

ImproperCTypes: also check 'exported' static variables

1ea05b1

Added the missing case for FFI-exposed pieces of code: static variables with the `no_mangle` or `export_name` annotations. This adds a new lint, which is managed by the rest of the ImproperCTypes architecture.

ImproperCTypes: don't consider packed reprs

f48dc94

`[repr(C,packed)]` structs shouldn't be considered FFI-safe

ImproperCTypes: misc. adaptations

5382587

smooth things out to avoid conflicts with rust-lang/compiler-builtins#1006 which has at time of writing not made it into rust-lang/rust's main branch

ImproperCTypes: rename associated tests

59de843

Rustc is trying to shift away from test names that are just issue numbers, so we add this as part of its effort, since this commit chain is moving and rewriting these files anyway. The directory containing all tests is also renamed.

niacdoial force-pushed the linting-ptrdyn-ffi branch from 22b00d0 to 59de843 Compare August 30, 2025 19:50

		// but for some reason one can just go and write function pointers like that:
		// `type Foo = extern "C" fn(::std::ffi::CStr);`

		#[allow(non_snake_case)]
		mod CTypesVisitorStateFlags {

lint ImproperCTypes: overhaul (take 2 of "better handling of indirections") #134697

Are you sure you want to change the base?

lint ImproperCTypes: overhaul (take 2 of "better handling of indirections") #134697

Uh oh!

Conversation

niacdoial commented Dec 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

niacdoial commented Dec 23, 2024

Uh oh!

niacdoial commented Dec 23, 2024

Uh oh!

workingjubilee commented Dec 24, 2024

Uh oh!

workingjubilee commented Dec 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

workingjubilee commented Jan 4, 2025

Uh oh!

niacdoial commented Jan 5, 2025

Uh oh!

bors commented Jan 15, 2025

Uh oh!

niacdoial commented Jan 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

bors commented Jan 23, 2025

Uh oh!

This comment has been minimized.

workingjubilee commented Jan 31, 2025

Uh oh!

workingjubilee commented Jan 31, 2025

Uh oh!

workingjubilee commented Feb 20, 2025

Uh oh!

workingjubilee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

niacdoial commented Feb 27, 2025

Uh oh!

rustbot commented Aug 29, 2025

Uh oh!

This comment has been minimized.

tgross35 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

niacdoial commented Dec 23, 2024 •

edited

Loading

workingjubilee commented Dec 24, 2024 •

edited

Loading

niacdoial commented Jan 18, 2025 •

edited

Loading

tgross35 left a comment •

edited

Loading