From 3297e9f1080cf4d80319721d820e43ac6bddf239 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Thu, 27 Feb 2025 16:30:18 -0800 Subject: [PATCH] Review checklist: suggest checking insertions expected to be new. (#7245) Suggest checking that PRs assert that insertions into sets or maps expected to be adding new values didn't actually just replace some existing value. Bug #7048 and its several duplicates would have been caught sooner if the insertion of the new spill temporary into the `spilled_composites` table had asserted that there was no existing spill variable for that expression. --- docs/review-checklist.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/review-checklist.md b/docs/review-checklist.md index 5dcee99b38..b422b8f585 100644 --- a/docs/review-checklist.md +++ b/docs/review-checklist.md @@ -34,6 +34,9 @@ satisfying way to address in a more robust way. - [ ] If your change iterates over a collection, did you ensure the order of iteration was deterministic? Using `HashMap` and `HashSet` is fine, as long as you don't iterate over it. +- [ ] If you insert elements into a set or map that you expect are not + already present, did you make an assertion about `insert`'s + return value? ### WGSL Extensions