Skip to content

[libc++][string] Remove potential non-trailing 0-length array #108867

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

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Sep 16, 2024

It is a violation of the standard to use 0 length arrays, especially when not at the end of a structure (not a FAM GNU extension). Compiler generally accept it, but it's probably better to have a conforming implementation.

This is a re-application of #105865 which was reverted in 72cfc74 because it broke the data formatters. A LLDB patch has since been landed that should make this a non-issue.

It is a violation of the standard to use 0 length arrays, especially
when not at the end of a structure (not a FAM GNU extension). Compiler
generally accept it, but it's probably better to have a conforming
implementation.

This is a re-application of llvm#105865 which was reverted in 72cfc74
because it broke the data formatters. A LLDB patch has since been
landed that should make this a non-issue.
@ldionne ldionne requested a review from a team as a code owner September 16, 2024 18:10
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 16, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

It is a violation of the standard to use 0 length arrays, especially when not at the end of a structure (not a FAM GNU extension). Compiler generally accept it, but it's probably better to have a conforming implementation.

This is a re-application of #105865 which was reverted in 72cfc74 because it broke the data formatters. A LLDB patch has since been landed that should make this a non-issue.


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

1 Files Affected:

  • (modified) libcxx/include/string (+10-2)
diff --git a/libcxx/include/string b/libcxx/include/string
index 76359022f3650c..fdb189016bfbac 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -749,6 +749,14 @@ struct __can_be_converted_to_string_view
 struct __uninitialized_size_tag {};
 struct __init_with_sentinel_tag {};
 
+template <size_t _PaddingSize>
+struct __padding {
+  char __padding_[_PaddingSize];
+};
+
+template <>
+struct __padding<0> {};
+
 template <class _CharT, class _Traits, class _Allocator>
 class basic_string {
 private:
@@ -853,7 +861,7 @@ private:
 
   struct __short {
     value_type __data_[__min_cap];
-    unsigned char __padding_[sizeof(value_type) - 1];
+    _LIBCPP_NO_UNIQUE_ADDRESS __padding<sizeof(value_type) - 1> __padding_;
     unsigned char __size_    : 7;
     unsigned char __is_long_ : 1;
   };
@@ -905,7 +913,7 @@ private:
       unsigned char __is_long_ : 1;
       unsigned char __size_    : 7;
     };
-    char __padding_[sizeof(value_type) - 1];
+    _LIBCPP_NO_UNIQUE_ADDRESS __padding<sizeof(value_type) - 1> __padding_;
     value_type __data_[__min_cap];
   };
 

@ldionne ldionne merged commit d95597d into llvm:main Sep 16, 2024
61 of 63 checks passed
@ldionne ldionne deleted the review/fix-0-length-array-string branch September 16, 2024 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants