testenv/smallVector: avoid aggressive loop optimization warnings

[davvid]

gcc (Debian 15.3.0-1) 15.3.0 and warns about undefined behavior:

In copy constructor ‘TestStruct::TestStruct(const TestStruct&)’,
  inlined from ‘void std::_Construct(_Tp*, _Args&& ...)
          [with _Tp = TestStruct; _Args = {const TestStruct&}]’
          at /usr/include/c++/15/bits/stl_construct.h:133:7,
  ...
  inlined from ‘void TfSmallVector<T, N>::insert(iterator, ForwardIterator, ForwardIterator)
          [with ForwardIterator = const TestStruct*; T = TestStruct; unsigned int N = 15]’
          at pxr/base/tf/smallVector.h:579:36,
  inlined from ‘void TfSmallVector<T, N>::insert(iterator, std::initializer_list<_Tp>)
          [with T = TestStruct; unsigned int N = 15]’
          at pxr/base/tf/smallVector.h:588:15,
  inlined from ‘void testInsertion()’ at pxr/base/tf/testenv/smallVector.cpp:1212:21:
pxr/base/tf/testenv/smallVector.cpp:1036:16:
  warning: iteration 4294967295 invokes undefined behavior [-Waggressive-loop-optimizations]
 1036 |         counter++;
      |         ~~~~~~~^~

Avoid undefined behavior by switching the static counter to uint32_t.

Description of Change(s)

Fixes Issue(s)

• Avoids detected Undefined Behavior and possible miscompilation due to optimizations when building with GCC.

Checklist

• I have created this PR based on the dev branch
• I have followed the coding conventions
• I have added unit tests that exercise this functionality (Reference:
  testing guidelines)
• I have verified that all unit tests pass with the proposed changes
• I have submitted a signed Contributor License Agreement (Reference:
  Contributor License Agreement instructions)

[nvmkuruc]

@davvid Was this only observed in the small vector test? I'm wondering if this can be more locally addressed in the test rather than add the compiler option for the whole project.

[jesschimein]

Filed as internal issue #USD-12504

❗ Please make sure that a signed CLA has been submitted!

(This is an automated message. See here for more information.)

[davvid]

Yes, it was only observed there. If we're okay with signed overflow being considered UB then I could try to take a stab at using pragmas in the test .cpp to disable the warning for just that translation unit. I tried adding static assertions (in hope that the compiler would pickup the fact that it doesn't overflow in practice) but that didn't work when I was messing with it.

[nvmkuruc]

@nvidia-jomiller has had success using TF_AXIOM or TF_DEV_AXIOM if I recall correctly for a similar issue.

[davvid]

@nvmkuruc this PR has been updated to use #pragma GCC diagnostic to disable -Waggressive-loop-optimizations for a narrow block of code within the test instead of enabling -fwrapv.

TF_AXIOM / TF_DEV_AXIOM did not seem to be able to silence the warning.

[nvmkuruc]

@davvid Pixar has a pattern for pragmas to minimize architecture specific behavior in downstream code that might be worth considering.

https://github.com/PixarAnimationStudios/OpenUSD/blob/dev/pxr/base/arch/pragmas.h

[nvmkuruc]

@davvid Thinking about this a bit more, would it just be better to make static int counter static size_t counter instead? Presumably making it unsigned would avoid the issue? I don't have GCC 15 otherwise I'd test to verify.

[davvid]

Ahh, thanks for the ARCH_PRAGMA point. I grepped the code and copied the pragma structure from another file so having them both using the arch pragma macros across the board would be best.

That said, the idea to switch the counter to be a size_t is a great idea. I'll give that a try later and update this PR if that does the trick.

[davvid]

Yup, switching to an unsigned uint32_t worked. cheers