fix(IWYU): force std:: or <> for max, min, copy - #411
Conversation
Sorry, something went wrong.
suppress false positive on direct initialization (`int max(0);`)
There was a problem hiding this comment.
Pull request overview
This PR fixes a false positive in the Include What You Use (IWYU) checker by preventing direct initialization syntax (like int max(0);) from being incorrectly flagged as requiring #include <algorithm>. The fix moves copy, max, and min from the general template detection pattern to a specialized pattern that only matches when these functions are:
- Explicitly qualified with std:: (e.g., std::max(a, b)), OR
- Used with template arguments (e.g., max<int>(a, b))
Key Changes
- Removes copy, max, and min from the _HEADERS_MAYBE_TEMPLATES tuple to prevent the generic pattern from matching them
- Adds a specialized regex pattern that only matches fully qualified or explicitly templated uses of these three functions
- Adds a test case to verify that direct initialization syntax doesn't trigger false positives
- Minor comment improvement for consistency
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cpplint.py | Removes copy, max, min from generic template list and adds specialized pattern with stricter matching rules to avoid false positives |
| cpplint_unittest.py | Adds test case verifying that direct initialization syntax (int max(0)) doesn't trigger IWYU warnings |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
| """, | ||
| "Add #include <algorithm> for min [build/include_what_you_use] [4]", | ||
| ) | ||
| self.TestIncludeWhatYouUse("int max(0), copy(max), min();", "") |
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
The test case verifies that false positives are suppressed, but there's no corresponding test to verify that legitimate uses of std::max(), std::min(), or std::copy() still trigger the include warning. Consider adding test cases like:
self.TestIncludeWhatYouUse("int x = std::max(a, b);",
"Add #include <algorithm> for max [build/include_what_you_use] [4]")
self.TestIncludeWhatYouUse("std::copy(src.begin(), src.end(), dst.begin());",
"Add #include <algorithm> for copy [build/include_what_you_use] [4]")This ensures the fix doesn't break detection of actual usage.
Sorry, something went wrong.
There was a problem hiding this comment.
How was this resolved? Do those tests already exist or are they not worth adding?
Sorry, something went wrong.
There was a problem hiding this comment.
Indeed, I didn't think they were worth adding—the max and copy regexes are the same as the already-tested one for min, and just like how regexes for max and copy weren't already there, throughout the unit tests each part of the relevant regex pattern is only tested once. Sorry for resolving without leaving this comment.
Sorry, something went wrong.
cclauss
left a comment
There was a problem hiding this comment.
RSLGTM
Sorry, something went wrong.
|
Thanks—had to search up what a RS is :) Could you also check out #376? That's the one I really want to get over with. |
Sorry, something went wrong.
suppress false positive on direct initialization (int max(0);)
Fixes #296