Skip to content

Commit b7426e7

Browse files
wisniewskijtjzel
authored andcommitted
Make clang-tidy work in parallel (#9108)
## Summary Current clang-tidy configuration is painfully slow (because it runs synchronously on one thread) + from what I understand, passing .h files in a way it's currently done, is not suggested and it can cause some problems with errors not being displayed properly (some errors in .h files are displayed as warnings, which can cause problems). I changed the configuration to use "run-clang-tidy" script which enables us to fix both of those issues. Additionally I fixed all the issues that are now caught in the .h files by the script. ## Test plan in both `packages/react-native-worklets` and `packages/react-native-reanimated` run `yarn lint:clang-tidy` --------- Co-authored-by: Tomasz Żelawski <tzelawski@gmail.com>
1 parent 68830ae commit b7426e7

19 files changed

Lines changed: 108 additions & 103 deletions

.github/workflows/lint-clang-tidy.yml

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ on:
77
- main
88
paths:
99
- '.github/workflows/lint-clang-tidy.yml'
10+
- 'scripts/clang-tidy-lint.sh'
1011

1112
merge_group:
1213
branches:
@@ -15,6 +16,7 @@ on:
1516
pull_request:
1617
paths:
1718
- '.github/workflows/lint-clang-tidy.yml'
19+
- 'scripts/clang-tidy-lint.sh'
1820
workflow_call:
1921
workflow_dispatch:
2022

@@ -30,12 +32,9 @@ jobs:
3032

3133
- name: Install monorepo node dependencies
3234
run: yarn install --immutable
33-
- name: Build Worklets package
34-
run: yarn workspace react-native-worklets build
35-
- name: Build Reanimated package
36-
run: yarn workspace react-native-reanimated build
3735

3836
- name: Clang-tidy lint Worklets
3937
run: yarn workspace react-native-worklets lint:clang-tidy
38+
4039
- name: Clang-tidy lint Reanimated
4140
run: yarn workspace react-native-reanimated lint:clang-tidy

packages/react-native-reanimated/Common/cpp/reanimated/CSS/utils/DelayedItemsManager.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ bool DelayedItemComparator<TValue>::operator()(const DelayedItem<TValue> &lhs, c
1818
}
1919

2020
template <typename TValue>
21-
void DelayedItemsManager<TValue>::add(const double timestamp, const TValue value) {
21+
void DelayedItemsManager<TValue>::add(const double timestamp, const TValue &value) {
2222
auto result = itemsSet_.emplace(timestamp, value);
2323
if (result.second) {
2424
itemsMap_[result.first->value] = result.first;
@@ -38,7 +38,7 @@ typename DelayedItemsManager<TValue>::Item DelayedItemsManager<TValue>::pop() {
3838
}
3939

4040
template <typename TValue>
41-
bool DelayedItemsManager<TValue>::remove(const TValue value) {
41+
bool DelayedItemsManager<TValue>::remove(const TValue &value) {
4242
auto it = itemsMap_.find(value);
4343

4444
if (it == itemsMap_.end()) {

packages/react-native-reanimated/Common/cpp/reanimated/CSS/utils/DelayedItemsManager.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ class DelayedItemsManager {
3434
ItemMap itemsMap_;
3535

3636
public:
37-
void add(double timestamp, TValue value);
37+
void add(double timestamp, const TValue &value);
3838
Item pop();
39-
bool remove(TValue value);
39+
bool remove(const TValue &value);
4040
const Item &top() const;
4141
bool empty() const;
4242
size_t size() const;

packages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy_Experimental.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ std::optional<MountingTransaction> LayoutAnimationsProxy_Experimental::pullTrans
3232
ShadowViewMutationList filteredMutations;
3333
auto rootChildCount = static_cast<int>(lightNodes_[surfaceId]->children.size());
3434
const std::vector<std::shared_ptr<MutationNode>> roots;
35-
const bool isInTransition = transitionState_;
35+
const bool isInTransition = static_cast<bool>(transitionState_);
3636

3737
if (isInTransition) {
3838
updateLightTree(propsParserContext, mutations, filteredMutations);

packages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy_Experimental.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ struct LayoutAnimationsProxy_Experimental : public LayoutAnimationsProxyCommon,
4343
mutable Tag transitionTag_;
4444
mutable double transitionProgress_;
4545
mutable bool transitionUpdated_;
46-
mutable TransitionState transitionState_ = NONE;
46+
mutable TransitionState transitionState_ = TransitionState::NONE;
4747
mutable SurfaceId transitioningSurfaceId_ = -1;
4848
mutable std::unordered_map<SurfaceId, std::shared_ptr<LightNode>> topScreen;
4949
mutable int containerTag_ = 10000002;

packages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy_Legacy.cpp

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ std::optional<SurfaceId> LayoutAnimationsProxy_Legacy::endLayoutAnimation(int ta
124124

125125
auto node = nodeForTag_[tag];
126126
auto mutationNode = std::static_pointer_cast<MutationNode>(node);
127-
mutationNode->state = DEAD;
127+
mutationNode->state = ExitingState_Legacy::DEAD;
128128
auto &[deadNodes] = surfaceContext_[surfaceId];
129129
deadNodes.insert(mutationNode);
130130

@@ -172,7 +172,7 @@ void LayoutAnimationsProxy_Legacy::parseRemoveMutations(
172172
}
173173
}
174174
if (!deletedViews.contains(mutation.oldChildShadowView.tag)) {
175-
mutationNode->state = MOVED;
175+
mutationNode->state = ExitingState_Legacy::MOVED;
176176
movedViews.insert_or_assign(mutation.oldChildShadowView.tag, -1);
177177
}
178178
nodeForTag_[tag] = mutationNode;
@@ -245,11 +245,11 @@ void LayoutAnimationsProxy_Legacy::handleRemovals(
245245
if (!startAnimationsRecursively(node, true, shouldAnimate, false, filteredMutations)) {
246246
filteredMutations.push_back(node->mutation);
247247
node->unflattenedParent->removeChildFromUnflattenedTree(node); //???
248-
if (node->state != MOVED) {
248+
if (node->state != ExitingState_Legacy::MOVED) {
249249
maybeCancelAnimation(node->tag);
250250
filteredMutations.push_back(ShadowViewMutation::DeleteMutation(node->mutation.oldChildShadowView));
251251
nodeForTag_.erase(node->tag);
252-
node->state = DELETED;
252+
node->state = ExitingState_Legacy::DELETED;
253253
#ifdef LAYOUT_ANIMATIONS_LOGS
254254
LOG(INFO) << "delete " << node->tag << std::endl;
255255
#endif
@@ -258,7 +258,7 @@ void LayoutAnimationsProxy_Legacy::handleRemovals(
258258
}
259259

260260
for (const auto &node : deadNodes) {
261-
if (node->state != DELETED) {
261+
if (node->state != ExitingState_Legacy::DELETED) {
262262
endAnimationsRecursively(node, filteredMutations);
263263
maybeDropAncestors(node->unflattenedParent, node, filteredMutations);
264264
}
@@ -429,12 +429,12 @@ void LayoutAnimationsProxy_Legacy::endAnimationsRecursively(
429429
const std::shared_ptr<MutationNode> &node,
430430
ShadowViewMutationList &mutations) const {
431431
maybeCancelAnimation(node->tag);
432-
node->state = DELETED;
432+
node->state = ExitingState_Legacy::DELETED;
433433
// iterate from the end, so that children
434434
// with higher indices appear first in the mutations list
435435
for (auto it = node->unflattenedChildren.rbegin(); it != node->unflattenedChildren.rend(); it++) {
436436
auto &subNode = *it;
437-
if (subNode->state != DELETED) {
437+
if (subNode->state != ExitingState_Legacy::DELETED) {
438438
endAnimationsRecursively(subNode, mutations);
439439
}
440440
}
@@ -457,11 +457,11 @@ void LayoutAnimationsProxy_Legacy::maybeDropAncestors(
457457

458458
auto node = std::static_pointer_cast<MutationNode>(parent);
459459

460-
if (node->children.size() == 0 && node->state != ANIMATING) {
460+
if (node->children.size() == 0 && node->state != ExitingState_Legacy::ANIMATING) {
461461
nodeForTag_.erase(node->tag);
462462
cleanupMutations.push_back(node->mutation);
463463
maybeCancelAnimation(node->tag);
464-
node->state = DELETED;
464+
node->state = ExitingState_Legacy::DELETED;
465465
#ifdef LAYOUT_ANIMATIONS_LOGS
466466
LOG(INFO) << "delete " << node->tag << std::endl;
467467
#endif
@@ -492,7 +492,7 @@ bool LayoutAnimationsProxy_Legacy::startAnimationsRecursively(
492492
bool hasAnimatedChildren = false;
493493

494494
shouldRemoveSubviewsWithoutAnimations =
495-
shouldRemoveSubviewsWithoutAnimations && (!hasExitAnimation || node->state == MOVED);
495+
shouldRemoveSubviewsWithoutAnimations && (!hasExitAnimation || node->state == ExitingState_Legacy::MOVED);
496496
std::vector<std::shared_ptr<MutationNode>> toBeRemoved;
497497

498498
// iterate from the end, so that children
@@ -503,8 +503,8 @@ bool LayoutAnimationsProxy_Legacy::startAnimationsRecursively(
503503
LOG(INFO) << "child " << subNode->tag << " "
504504
<< " " << shouldAnimate << " " << shouldRemoveSubviewsWithoutAnimations << std::endl;
505505
#endif
506-
if (subNode->state != UNDEFINED && subNode->state != MOVED) {
507-
if (shouldAnimate && subNode->state != DEAD) {
506+
if (subNode->state != ExitingState_Legacy::UNDEFINED && subNode->state != ExitingState_Legacy::MOVED) {
507+
if (shouldAnimate && subNode->state != ExitingState_Legacy::DEAD) {
508508
hasAnimatedChildren = true;
509509
} else {
510510
endAnimationsRecursively(subNode, mutations);
@@ -516,29 +516,29 @@ bool LayoutAnimationsProxy_Legacy::startAnimationsRecursively(
516516
LOG(INFO) << "child " << subNode->tag << " start animations returned true " << std::endl;
517517
#endif
518518
hasAnimatedChildren = true;
519-
} else if (subNode->state == MOVED) {
519+
} else if (subNode->state == ExitingState_Legacy::MOVED) {
520520
mutations.push_back(subNode->mutation);
521521
toBeRemoved.push_back(subNode);
522522
} else if (shouldRemoveSubviewsWithoutAnimations) {
523523
maybeCancelAnimation(subNode->tag);
524524
mutations.push_back(subNode->mutation);
525525
toBeRemoved.push_back(subNode);
526-
subNode->state = DELETED;
526+
subNode->state = ExitingState_Legacy::DELETED;
527527
nodeForTag_.erase(subNode->tag);
528528
#ifdef LAYOUT_ANIMATIONS_LOGS
529529
LOG(INFO) << "delete " << subNode->tag << std::endl;
530530
#endif
531531
mutations.push_back(ShadowViewMutation::DeleteMutation(subNode->mutation.oldChildShadowView));
532532
} else {
533-
subNode->state = WAITING;
533+
subNode->state = ExitingState_Legacy::WAITING;
534534
}
535535
}
536536

537537
for (auto &subNode : toBeRemoved) {
538538
node->removeChildFromUnflattenedTree(subNode);
539539
}
540540

541-
if (node->state == MOVED) {
541+
if (node->state == ExitingState_Legacy::MOVED) {
542542
auto replacement = std::make_shared<Node>(*node);
543543
for (const auto &subNode : node->children) {
544544
subNode->parent = replacement;
@@ -553,7 +553,7 @@ bool LayoutAnimationsProxy_Legacy::startAnimationsRecursively(
553553
bool wantAnimateExit = hasExitAnimation || hasAnimatedChildren;
554554

555555
if (hasExitAnimation) {
556-
node->state = ANIMATING;
556+
node->state = ExitingState_Legacy::ANIMATING;
557557
startExitingAnimation(node->tag, node->mutation);
558558
} else {
559559
layoutAnimationsManager_->clearLayoutAnimationConfig(node->tag);

packages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy_Legacy.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class ReanimatedModuleProxy;
2424

2525
using namespace facebook;
2626

27-
typedef enum ExitingState_Legacy : std::uint8_t {
27+
typedef enum class ExitingState_Legacy : std::uint8_t {
2828
UNDEFINED = 1,
2929
WAITING = 2,
3030
ANIMATING = 4,
@@ -60,7 +60,7 @@ struct Node {
6060
*/
6161
struct MutationNode : public Node {
6262
ShadowViewMutation mutation;
63-
ExitingState_Legacy state = UNDEFINED;
63+
ExitingState_Legacy state = ExitingState_Legacy::UNDEFINED;
6464
explicit MutationNode(ShadowViewMutation &mutation) : Node(mutation.oldChildShadowView.tag), mutation(mutation) {}
6565
MutationNode(ShadowViewMutation &mutation, Node &&node) : Node(std::move(node)), mutation(mutation) {}
6666
bool isMutationNode() override;

packages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsUtils.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
namespace reanimated {
1515

16-
enum BeforeOrAfter : std::uint8_t { BEFORE = 0, AFTER = 1 };
16+
enum BeforeOrAfter : std::uint8_t { BEFORE = 0, AFTER = 1 }; // NOLINT
1717

1818
struct Rect {
1919
double width, height;
@@ -66,15 +66,15 @@ typedef enum class ExitingState : std::uint8_t {
6666

6767
struct MutationNode;
6868

69-
enum TransitionState : std::uint8_t {
69+
enum class TransitionState : std::uint8_t {
7070
NONE = 0,
7171
START = 1,
7272
ACTIVE = 2,
7373
END = 3,
7474
CANCELLED = 4,
7575
};
7676

77-
enum Intent : std::uint8_t {
77+
enum class Intent : std::uint8_t {
7878
NO_INTENT = 0,
7979
TO_MOVE = 1,
8080
TO_DELETE = 2,

packages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/SharedTransitions.cpp

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include <react/renderer/components/scrollview/ScrollViewState.h>
22
#include <reanimated/LayoutAnimations/LayoutAnimationsProxy_Experimental.h>
3+
#include <reanimated/LayoutAnimations/LayoutAnimationsUtils.h>
34
#include <reanimated/Tools/ReanimatedSystraceSection.h>
45
#ifndef ANDROID
56
#if __has_include(<react/renderer/components/rnscreens/Props.h>)
@@ -62,9 +63,11 @@ void LayoutAnimationsProxy_Experimental::findSharedElementsOnScreen(
6263
auto newTransform = parseParentTransforms(node, absolutePositions);
6364
const auto &parent = node->parent.lock();
6465
react_native_assert(parent && "Parent node is nullptr");
65-
transform[index] = std::move(newTransform);
66-
snapshot[index] = copy;
67-
parentTag[index] = parent->current.tag;
66+
67+
int indexNum = static_cast<int>(index);
68+
transform[indexNum] = std::move(newTransform);
69+
snapshot[indexNum] = copy;
70+
parentTag[indexNum] = parent->current.tag;
6871

6972
if (parentTag[BEFORE] && parentTag[AFTER]) {
7073
transitions_.emplace_back(sharedTag, transition);
@@ -88,11 +91,11 @@ void LayoutAnimationsProxy_Experimental::handleProgressTransition(
8891
}
8992
transitionUpdated_ = false;
9093

91-
if (!mutations.empty() || !transitionState_) {
94+
if (!mutations.empty() || !static_cast<bool>(transitionState_)) {
9295
return;
9396
}
9497

95-
if (transitionState_ == START) {
98+
if (transitionState_ == TransitionState::START) {
9699
auto root = lightNodes_[surfaceId];
97100
auto beforeTopScreen = topScreen[surfaceId];
98101
auto afterTopScreen = lightNodes_[transitionTag_];
@@ -119,7 +122,7 @@ void LayoutAnimationsProxy_Experimental::handleProgressTransition(
119122
startProgressTransition(containerTag, before, after, surfaceId);
120123
}
121124
}
122-
} else if (transitionState_ == ACTIVE) {
125+
} else if (transitionState_ == TransitionState::ACTIVE) {
123126
for (auto tag : activeTransitions_) {
124127
auto layoutAnimation = layoutAnimations_[tag];
125128
auto &updateMap = surfaceManager.getUpdateMap(layoutAnimation.finalView.surfaceId);
@@ -153,23 +156,23 @@ void LayoutAnimationsProxy_Experimental::handleProgressTransition(
153156
}
154157
}
155158

156-
if (transitionState_ == START) {
157-
transitionState_ = ACTIVE;
158-
} else if (transitionState_ == END || transitionState_ == CANCELLED) {
159+
if (transitionState_ == TransitionState::START) {
160+
transitionState_ = TransitionState::ACTIVE;
161+
} else if (transitionState_ == TransitionState::END || transitionState_ == TransitionState::CANCELLED) {
159162
for (auto tag : activeTransitions_) {
160163
sharedContainersToRemove_.push_back(tag);
161164
tagsToRestore_.push_back(restoreMap_[tag][AFTER]);
162-
if (transitionState_ == CANCELLED) {
165+
if (transitionState_ == TransitionState::CANCELLED) {
163166
tagsToRestore_.push_back(restoreMap_[tag][BEFORE]);
164167
}
165168
}
166-
if (transitionState_ == END) {
169+
if (transitionState_ == TransitionState::END) {
167170
topScreen[surfaceId] = lightNodes_[transitionTag_];
168171
synchronized_ = false;
169172
}
170173
sharedTransitionManager_->containerTags_.clear();
171174
activeTransitions_.clear();
172-
transitionState_ = NONE;
175+
transitionState_ = TransitionState::NONE;
173176
}
174177
}
175178

@@ -278,8 +281,9 @@ void LayoutAnimationsProxy_Experimental::hideTransitioningViews(
278281
ShadowViewMutationList &filteredMutations,
279282
const PropsParserContext &propsParserContext) const {
280283
for (auto &[sharedTag, transition] : transitions_) {
281-
const auto &shadowView = transition.snapshot[index];
282-
const auto &parentTag = transition.parentTag[index];
284+
int indexNum = static_cast<int>(index);
285+
const auto &shadowView = transition.snapshot[indexNum];
286+
const auto &parentTag = transition.parentTag[indexNum];
283287
auto m = ShadowViewMutation::UpdateMutation(
284288
shadowView, cloneViewWithoutOpacity(shadowView, propsParserContext), parentTag);
285289
filteredMutations.push_back(m);
@@ -303,11 +307,11 @@ std::optional<SurfaceId> LayoutAnimationsProxy_Experimental::onTransitionProgres
303307
// transitions (maybe that's ok?)
304308
if (!isClosing && !isGoingForward && !isAndroid) {
305309
transitionProgress_ = progress;
306-
if (transitionState_ == NONE && progress < 1) {
307-
transitionState_ = START;
310+
if (transitionState_ == TransitionState::NONE && progress < 1) {
311+
transitionState_ = TransitionState::START;
308312
transitionTag_ = tag;
309-
} else if (transitionState_ == ACTIVE && progress == 1) {
310-
transitionState_ = END;
313+
} else if (transitionState_ == TransitionState::ACTIVE && progress == 1) {
314+
transitionState_ = TransitionState::END;
311315
}
312316
const auto &node = lightNodes_[tag];
313317
react_native_assert(node && "LightNode is nullptr");
@@ -320,8 +324,8 @@ std::optional<SurfaceId> LayoutAnimationsProxy_Experimental::onTransitionProgres
320324

321325
std::optional<SurfaceId> LayoutAnimationsProxy_Experimental::onGestureCancel() {
322326
auto lock = std::unique_lock<std::recursive_mutex>(mutex);
323-
if (transitionState_) {
324-
transitionState_ = CANCELLED;
327+
if (static_cast<bool>(transitionState_)) {
328+
transitionState_ = TransitionState::CANCELLED;
325329
transitionUpdated_ = true;
326330
react_native_assert(transitioningSurfaceId_ != -1 && "Cancelling non-observed transition");
327331

0 commit comments

Comments
 (0)