Fix deprecation warnings in FastWindingNumbersForSoups.h - #2476

I'm sorry this is still changing every line in the diff. Maybe it's tabs spaces or newlines. But I can't figure out what's actually changed in the file.

Is there anyway to filter the changes to not touch every line? Maybe it's a newline thing?

Actually I dont change any code before line 1148.
Those differences can not be seen in the local repository, but only after pushed to github.

Reviewed and verified — this looks correct and behavior-preserving. Details of how I checked:

Scope of the diff. Of the ~822 changed lines, the large block (the non-SSE __VM_SIMDFunc__ SIMD fallback, ~800 lines) is a pure CRLF→LF line-ending normalization: diff -w between pre-PR and post-PR of that region is byte-identical. It contains no is_pod/std::iterator. The three substantive hunks are the deprecation fixes.

Semantic-equivalence audit

1. std::iterator<std::random_access_iterator_tag, T, exint> on UT_Array::base_iterator → explicit typedefs. The original also shadowed the base's pointer/reference with typedef IT* pointer; typedef IT& reference;, so the effective members were {category=random_access, value_type=T, difference_type=exint, pointer=IT*, reference=IT&}. The PR declares {iterator_category=random_access_iterator_tag, value_type=IT, difference_type=exint, pointer=IT*, reference=IT&}. The only difference vs. the original is value_type (was T, now IT). This is benign — nothing in the header (no iterator_traits/algorithm) consumes base_iterator::value_type, pointer/reference/difference_type are unchanged, and IT is precisely the element type the iterator dereferences to (myCurrent is IT*, operator* returns IT&), so value_type=IT is if anything more self-consistent. No functional change.

2. isPOD(): std::is_pod<T>::value → is_standard_layout && is_trivially_default_constructible && is_trivially_copyable && is_trivially_move_assignable && is_trivially_destructible. Since is_pod == is_trivial && is_standard_layout and is_trivial == is_trivially_copyable && is_trivially_default_constructible (with trivially_move_assignable/destructible already implied by trivially_copyable), this is equivalent to is_trivial<T> && is_standard_layout<T>. Correct.

3. Box copy-ctor static_assert: both is_pod<...> uses expanded to is_standard_layout && is_trivially_copyable && is_trivially_default_constructible (= is_trivial && is_standard_layout = is_pod), preserving the POD(Box) || !POD(T) logic. Correct.

No accidental logic changes beyond the mechanical deprecation replacements.

Build + test (header-only, so the changed header is actually compiled).
g++ -std=c++20 -O2 -pthread -Wall -Wdeprecated-declarations ... tests/main.cpp tests/include/igl/fast_winding_number.cpp:

• Compiles cleanly under C++20 (exit 0).
• grep -iE "is_pod|std::iterator|deprecated" over the warning log → empty; the remaining warnings are all pre-existing/unrelated (-Wsign-compare, -Wunused-result, etc.).
• fwn_test "*winding*" → All tests passed (34 assertions in 2 test cases) — results unchanged.
• Post-PR grep -nE "is_pod|std::iterator" include/igl/FastWindingNumberForSoups.h → no matches; all such deprecations removed, none missed.

LGTM.

🤖 This review was written by Claude Code (Claude Opus 4.8) on behalf of the maintainer.

Line-ending issue: diagnosed, and it's already resolved in the current head

I dug into why this diff appeared to "change every line." Summary: it was a real whole-file CRLF→LF normalization, but the current PR head is clean — provided this is squash-merged (not rebase-merged).

Culprit. The original fix commit 1c061295 didn't only edit the deprecation lines — the whole file got normalized CRLF→LF on save:

• 1c061295 vs its parent: 415 insertions / 407 deletions
• parent had 399 CRLF lines, 1c061295 had 0.

That's the "every line changed" Alec saw. It wasn't visible in @cityadmirer's local checkout because core.autocrlf masks CR-at-eol; it only shows in GitHub's raw byte view. (Root cause: .gitattributes declares *.cpp/*.hpp as text but not *.h, so this header's line endings depend on each contributor's autocrlf setting.)

Why it's clean now. The subsequent merges of main restored the file to main's byte-for-byte content (CRLF preserved) with the three deprecation hunks reapplied. The tip-to-tip diff is now minimal:

• main...<head> = 16 ins / 7 del (GitHub shows 15+/7-)
• whole-file comparison main vs head = exactly 23 differing lines — only the real changes; the 399 CRLF lines are byte-identical to main.

Merge method matters:

• ✅ Squash (default): records a single commit whose diff is the clean 23-line change → git blame stays intact.
• ❌ Rebase-and-merge: would replay 1c061295 (the 415/407 whole-file flip) onto main and pollute git blame for ~399 lines.

Recommendation: squash-merge this PR. No changes to the branch are needed.

Separately, I'm opening a small PR to add *.h to .gitattributes and introduce a .git-blame-ignore-revs so this class of churn stops recurring.

🤖 This comment was written by Claude Code (Claude Opus 4.8) on behalf of the maintainer.

Correction to my comment above. Two of the byte-level readings in my previous comment were taken against the wrong branch tip (a local gh pr checkout had silently no-op'd, so git show HEAD: was reading an unrelated local branch, not this PR). Here are the verified facts, via git cat-file (no filters):

So, corrected:

• The current diff is already clean. main...<head> for FastWindingNumberForSoups.h = 16 insertions / 7 deletions — only the four real changes (#include <cassert>, the base_iterator typedefs, isPOD(), and the Box static_assert). Both sides are LF; there is no CRLF churn left in the PR. My earlier claim that main "retained CRLF" and the merges "restored" it was wrong — main is LF for this file, and so is this head.

• What Alec originally saw was real: the first fix commit 1c061295 was authored when the file on its base was still CRLF (its parent 8866f214 had 399 CRLF lines), and it normalized the whole file CRLF→LF (415/407) alongside the deprecation edits. main has since also moved to LF, so the tip-to-tip diff is now minimal.

• Merge method still matters, and the recommendation is unchanged: squash-merge (the default). Squash records exactly the clean 16/7 diff, so git blame attributes only those lines to this PR. Rebase-and-merge would replay 1c061295 — whose recorded diff is the whole-file 415/407 rewrite — into main's linear history, which is exactly the blame pollution to avoid. Squash sidesteps it entirely.

Net: no changes needed on the branch; squash-merge and blame stays intact. Apologies for the mixed-up readings in the prior comment.

🤖 This comment was written by Claude Code (Claude Opus 4.8) on behalf of the maintainer.