[llvm] [Transforms] Use range-based for loops (NFC) (PR #97195)

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 2 12:04:17 PDT 2024


================
@@ -226,9 +226,7 @@ static bool mergeConstants(Module &M) {
     // Now that we have figured out which replacements must be made, do them all
     // now.  This avoid invalidating the pointers in CMap, which are unneeded
     // now.
-    for (unsigned i = 0, e = SameContentReplacements.size(); i != e; ++i) {
-      GlobalVariable *Old = SameContentReplacements[i].first;
-      GlobalVariable *New = SameContentReplacements[i].second;
+    for (const auto &[Old, New] : SameContentReplacements) {
----------------
dwblaikie wrote:

Ah, looks like https://github.com/llvm/llvm-project/issues/47355 - std::pair is trivially copy constructible, but isn't trivially copy assignable (& so is not "trivially copyable") - and the warning used "trivially copyable" instead of "trivially copy constructible" for the test, so warned in this case - because it thought this was invoking a non-trivial copy constructor. So regardless of how small the pair is, it'd warn.

Do we have this warning on by default in llvm builds? Or is it turned on some other way in your dev environment?

It might be worth turning off this warning (maybe, if someone can be bothered, turning it off only if it's buggy in this way - we do have other warnings we have little compatibility tests for in cmake that check the warning doesn't have certain bugs before enabling it) due to this bug. 

https://github.com/llvm/llvm-project/pull/97195


More information about the llvm-commits mailing list