Skip to content

Convert some other local mutators and visitors to mutate_with and visit_with#8894

Open
alexreinking wants to merge 16 commits intomainfrom
alexreinking/convert-visitors
Open

Convert some other local mutators and visitors to mutate_with and visit_with#8894
alexreinking wants to merge 16 commits intomainfrom
alexreinking/convert-visitors

Conversation

@alexreinking
Copy link
Member

A concurrent complement to #8893

AddAtomicMutex.cpp is a little scary, actually.

@alexreinking alexreinking requested a review from abadams December 8, 2025 19:55
@abadams
Copy link
Member

abadams commented Dec 9, 2025

There are some clang-tidy things to fix, but I believe the failures are unrelated.

@alexreinking alexreinking force-pushed the alexreinking/convert-visitors branch 2 times, most recently from 60b6c8b to 0f767d3 Compare December 15, 2025 23:30
@alexreinking alexreinking force-pushed the alexreinking/convert-visitors branch from 0f767d3 to d4c32b8 Compare February 11, 2026 22:52
@alexreinking alexreinking force-pushed the alexreinking/convert-visitors branch from d4c32b8 to 11cb928 Compare February 13, 2026 04:38
@mcourteaux
Copy link
Contributor

As a brief note on pull requests like these where mutators and visitors are getting converted into the lambda version of them: When we go forward with the compiler profiling, we lose the RTTI ability to automatically infer the name of the mutator. So I recommend keeping all of them in at least a function with an equivalent descriptive name (like you did, it seems).

@alexreinking
Copy link
Member Author

As a brief note on pull requests like these where mutators and visitors are getting converted into the lambda version of them: When we go forward with the compiler profiling, we lose the RTTI ability to automatically infer the name of the mutator. So I recommend keeping all of them in at least a function with an equivalent descriptive name (like you did, it seems).

It would also be okay to add a const char * name argument to visit_with / mutate_with.

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.

3 participants