Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc] fix -Wmissing-braces #77345

Merged
merged 3 commits into from Jan 8, 2024

Conversation

nickdesaulniers
Copy link
Member

Fixes the following errors observed on the aarch64 fullbuild:

/home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/src/__support/HashTable/generic/bitmask_impl.inc:116:13:
error: suggest braces around initialization of subobject
[-Werror,-Wmissing-braces]
    return {static_cast<bitmask_t>(mask_available().word ^ repeat_byte(0x80))};
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            {                                                                }
In file included from
/home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/src/search/hdestroy.cpp:10:
/home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/src/__support/HashTable/table.h:336:41:
error: suggest braces around initialization of subobject
[-Werror,-Wmissing-braces]
  iterator end() const { return {0, 0, {0}, *this}; }
                                        ^
                                        {}

Link: https://lab.llvm.org/buildbot/#/builders/223/builds/33868/steps/6/logs/stdio
Link: #74506

Fixes the following errors observed on the aarch64 fullbuild:

    /home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/src/__support/HashTable/generic/bitmask_impl.inc:116:13:
    error: suggest braces around initialization of subobject
    [-Werror,-Wmissing-braces]
        return {static_cast<bitmask_t>(mask_available().word ^ repeat_byte(0x80))};
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                {                                                                }
    In file included from
    /home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/src/search/hdestroy.cpp:10:
    /home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/src/__support/HashTable/table.h:336:41:
    error: suggest braces around initialization of subobject
    [-Werror,-Wmissing-braces]
      iterator end() const { return {0, 0, {0}, *this}; }
                                            ^
                                            {}

Link: https://lab.llvm.org/buildbot/#/builders/223/builds/33868/steps/6/logs/stdio
Link: llvm#74506
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 8, 2024

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

Changes

Fixes the following errors observed on the aarch64 fullbuild:

/home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/src/__support/HashTable/generic/bitmask_impl.inc:116:13:
error: suggest braces around initialization of subobject
[-Werror,-Wmissing-braces]
    return {static_cast&lt;bitmask_t&gt;(mask_available().word ^ repeat_byte(0x80))};
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            {                                                                }
In file included from
/home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/src/search/hdestroy.cpp:10:
/home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/src/__support/HashTable/table.h:336:41:
error: suggest braces around initialization of subobject
[-Werror,-Wmissing-braces]
  iterator end() const { return {0, 0, {0}, *this}; }
                                        ^
                                        {}

Link: https://lab.llvm.org/buildbot/#/builders/223/builds/33868/steps/6/logs/stdio
Link: #74506


Full diff: https://github.com/llvm/llvm-project/pull/77345.diff

2 Files Affected:

  • (modified) libc/src/__support/HashTable/generic/bitmask_impl.inc (+1-1)
  • (modified) libc/src/__support/HashTable/table.h (+1-1)
diff --git a/libc/src/__support/HashTable/generic/bitmask_impl.inc b/libc/src/__support/HashTable/generic/bitmask_impl.inc
index b825cb5fbc44a8..d0cb342a60cd9c 100644
--- a/libc/src/__support/HashTable/generic/bitmask_impl.inc
+++ b/libc/src/__support/HashTable/generic/bitmask_impl.inc
@@ -113,7 +113,7 @@ struct Group {
   }
 
   LIBC_INLINE IteratableBitMask occupied() const {
-    return {static_cast<bitmask_t>(mask_available().word ^ repeat_byte(0x80))};
+    return {{static_cast<bitmask_t>(mask_available().word ^ repeat_byte(0x80))}};
   }
 };
 } // namespace internal
diff --git a/libc/src/__support/HashTable/table.h b/libc/src/__support/HashTable/table.h
index d70ca4d2338056..9b2f3beea9965e 100644
--- a/libc/src/__support/HashTable/table.h
+++ b/libc/src/__support/HashTable/table.h
@@ -333,7 +333,7 @@ struct HashTable {
     return {0, full_capacity() - available_slots,
             Group::load_aligned(&control(0)).occupied(), *this};
   }
-  iterator end() const { return {0, 0, {0}, *this}; }
+  iterator end() const { return {0, 0, {{0}}, *this}; }
 
   LIBC_INLINE ENTRY *find(const char *key) {
     uint64_t primary = oneshot_hash(key);

@nickdesaulniers
Copy link
Member Author

Strangely, I haven't been able to reproduce the build bots failures when trying to cross compile a fullbuild. My cmake:

$ cmake ../runtimes -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang -G Ninja -DLLVM_ENABLE_LLD=ON -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_RUNTIMES="libc" -DLLVM_LIBC_FULL_BUILD=ON -DLIBC_HDRGEN_EXE=$HDRGEN -DLIBC_TARGET_TRIPLE=aarch64-linux-gnu

Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we construct these explicitly? It's difficult to see what it's supposed to be doing with the brace initialization.

Copy link

github-actions bot commented Jan 8, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@nickdesaulniers
Copy link
Member Author

Could we construct these explicitly? It's difficult to see what it's supposed to be doing with the brace initialization.

as in

diff --git a/libc/src/__support/HashTable/table.h b/libc/src/__support/HashTable/table.h
index 9b2f3beea996..61670a5db0e3 100644
--- a/libc/src/__support/HashTable/table.h
+++ b/libc/src/__support/HashTable/table.h
@@ -333,7 +333,9 @@ public:
     return {0, full_capacity() - available_slots,
             Group::load_aligned(&control(0)).occupied(), *this};
   }
-  iterator end() const { return {0, 0, {{0}}, *this}; }
+  iterator end() const {
+    return {0, 0, IteratableBitMask{{0}}, *this};
+  }

?

@jhuber6
Copy link
Contributor

jhuber6 commented Jan 8, 2024

Could we construct these explicitly? It's difficult to see what it's supposed to be doing with the brace initialization.

as in

diff --git a/libc/src/__support/HashTable/table.h b/libc/src/__support/HashTable/table.h
index 9b2f3beea996..61670a5db0e3 100644
--- a/libc/src/__support/HashTable/table.h
+++ b/libc/src/__support/HashTable/table.h
@@ -333,7 +333,9 @@ public:
     return {0, full_capacity() - available_slots,
             Group::load_aligned(&control(0)).occupied(), *this};
   }
-  iterator end() const { return {0, 0, {{0}}, *this}; }
+  iterator end() const {
+    return {0, 0, IteratableBitMask{{0}}, *this};
+  }

?

I meant the iterator type, but yeah. It's just a little confusing to see so many brace initializers in one place, hence the warning.

@nickdesaulniers
Copy link
Member Author

I meant the iterator type,

Sorry, which of the two iterator types? The return value of end or the IteratableBitMask itself?

(When we can use c++20, designated initialization can be used to make this crystal clear).

@jhuber6
Copy link
Contributor

jhuber6 commented Jan 8, 2024

I meant the iterator type,

Sorry, which of the two iterator types? The return value of end or the IteratableBitMask itself?

(When we can use c++20, designated initialization can be used to make this crystal clear).

We're using {0} it implicitly create the iterator type rather than iterator{0} or whatever it is. I was just confused what it's actually trying to turn those arugment into since it just looks like any other uniform initialization

@nickdesaulniers
Copy link
Member Author

I meant the iterator type,

Sorry, which of the two iterator types? The return value of end or the IteratableBitMask itself?
(When we can use c++20, designated initialization can be used to make this crystal clear).

We're using {0} it implicitly create the iterator type rather than iterator{0} or whatever it is. I was just confused what it's actually trying to turn those arugment into since it just looks like any other uniform initialization

Done in a18bd3b PTAL

@nickdesaulniers nickdesaulniers merged commit c1023c5 into llvm:main Jan 8, 2024
4 checks passed
@nickdesaulniers nickdesaulniers deleted the aarch64_hashtable branch January 8, 2024 18:18
nickdesaulniers added a commit that referenced this pull request Jan 8, 2024
Similar to #77345, the buildbots are observing similar warnings for the
sse2
implementation.

llvm-project/libc/src/__support/HashTable/sse2/bitmask_impl.inc:36:13:
    error: suggest braces around initialization of subobject
    [-Werror,-Wmissing-braces]
    return {bitmask};
            ^~~~~~~
            {      }
llvm-project/libc/src/__support/HashTable/sse2/bitmask_impl.inc:45:13:
    error: suggest braces around initialization of subobject
    [-Werror,-Wmissing-braces]
    return {static_cast<uint16_t>(~mask_available().word)};
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            {                                            }

Link:
https://lab.llvm.org/buildbot/#/builders/163/builds/49350/steps/8/logs/stdio
Link: #74506
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
Fixes the following errors observed on the aarch64 fullbuild:


/home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/src/__support/HashTable/generic/bitmask_impl.inc:116:13:
    error: suggest braces around initialization of subobject
    [-Werror,-Wmissing-braces]
return {static_cast<bitmask_t>(mask_available().word ^
repeat_byte(0x80))};
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
{ }
    In file included from

/home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/src/search/hdestroy.cpp:10:

/home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/src/__support/HashTable/table.h:336:41:
    error: suggest braces around initialization of subobject
    [-Werror,-Wmissing-braces]
      iterator end() const { return {0, 0, {0}, *this}; }
                                            ^
                                            {}

Link:
https://lab.llvm.org/buildbot/#/builders/223/builds/33868/steps/6/logs/stdio
Link: llvm#74506
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
Similar to llvm#77345, the buildbots are observing similar warnings for the
sse2
implementation.

llvm-project/libc/src/__support/HashTable/sse2/bitmask_impl.inc:36:13:
    error: suggest braces around initialization of subobject
    [-Werror,-Wmissing-braces]
    return {bitmask};
            ^~~~~~~
            {      }
llvm-project/libc/src/__support/HashTable/sse2/bitmask_impl.inc:45:13:
    error: suggest braces around initialization of subobject
    [-Werror,-Wmissing-braces]
    return {static_cast<uint16_t>(~mask_available().word)};
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            {                                            }

Link:
https://lab.llvm.org/buildbot/#/builders/163/builds/49350/steps/8/logs/stdio
Link: llvm#74506
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants