Add static_assert(std::is_trivially_copyable<LinkedUniform>(),"")

Since we are using memcpy for LinkedUniform, it is desirable to utilize
compile time assertion to ensure that in future if anyone modifies POD
struct (and the class of data members of POD struct) and made it that no
longer memcopy-able, we would immediately caught at compile time.
std::is_trivially_copyable<>is exactly for this reason. In order to make
this work, the POD struct and any data it uses can not have user defined
copy constructor. The problem is that right now ANGLE is using
clang_use_chrome_plugins=true, and chrome-style generates warnings if
the complex struct (has more than 10 data members) does not define a
copy constructor, and that warning causes build failure with -Werror. So
clang_use_chrome_plugins=true and std::is_trivially_copyable have this
conflicting requirements that I can not apply both. This has been raised
to compiler team, but before we get a solution from them, if we have to
make a choice, I think the better choice is to disable
clang_use_chrome_plugins and apply std::is_trivially_copyable, since the
later is more critical to ensure safety, while chrome-style is mostly
trying to minimize the code size, but won't affect
correctness/robustness. This CL sets clang_use_chrome_plugins to false,
and removes the copy constructor and copy assignment operator from
BitSetT and LinkedUniform and added static assertion
is_trivially_copyable for LinkedUniform. Same thing applied to
ProgramInput as well.

In future once we have a better solution from compile team, we can
re-enable clang_use_chrome_plugins and disable only for structs that
requires is_trivially_copyable assertion.

Bug: b/275102061
Change-Id: If33415ea61deda568d855a7dd6a4fd6042058be5
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4799342
Reviewed-by: Roman Lavrov <romanl@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>

.gn

@@ -37,7 +37,7 @@ exec_script_whitelist = angle_dotfile_settings.exec_script_whitelist +
                         ]
 
 default_args = {
-  clang_use_chrome_plugins = true
+  clang_use_chrome_plugins = false
   build_angle_deqp_tests = true
   use_sysroot = true
 

src/common/bitset_utils.h

@@ -119,9 +119,6 @@ class BitSetT final
     constexpr explicit BitSetT(BitsT value);
     constexpr explicit BitSetT(std::initializer_list<ParamT> init);
 
-    constexpr BitSetT(const BitSetT &other);
-    constexpr BitSetT &operator=(const BitSetT &other);
-
     constexpr bool operator==(const BitSetT &other) const;
     constexpr bool operator!=(const BitSetT &other) const;
 
@@ -204,17 +201,6 @@ constexpr BitSetT<N, BitsT, ParamT>::BitSetT(std::initializer_list<ParamT> init)
     ASSERT(mBits == (mBits & Mask(N).bits()));
 }
 
-template <size_t N, typename BitsT, typename ParamT>
-constexpr BitSetT<N, BitsT, ParamT>::BitSetT(const BitSetT &other) : mBits(other.mBits)
-{}
-
-template <size_t N, typename BitsT, typename ParamT>
-constexpr BitSetT<N, BitsT, ParamT> &BitSetT<N, BitsT, ParamT>::operator=(const BitSetT &other)
-{
-    mBits = other.mBits;
-    return *this;
-}
-
 template <size_t N, typename BitsT, typename ParamT>
 constexpr bool BitSetT<N, BitsT, ParamT>::operator==(const BitSetT &other) const
 {
@@ -470,6 +456,7 @@ using BitSet16 = BitSetT<N, uint16_t>;
 
 template <size_t N>
 using BitSet32 = BitSetT<N, uint32_t>;
+static_assert(std::is_trivially_copyable<BitSet32<32>>(), "must be memcpy-able");
 
 template <size_t N>
 using BitSet64 = BitSetT<N, uint64_t>;
@@ -525,8 +512,6 @@ class BitSetArray final
     constexpr explicit BitSetArray(uint64_t value);
     constexpr explicit BitSetArray(std::initializer_list<param_type> init);
 
-    constexpr BitSetArray(const BitSetArray<N> &other);
-
     class Reference final
     {
       public:
@@ -660,7 +645,6 @@ class BitSetArray final
     }
 
     // Assignment operators
-    constexpr BitSetArray &operator=(const BitSetArray &other);
     constexpr BitSetArray &operator&=(const BitSetArray &other);
     constexpr BitSetArray &operator|=(const BitSetArray &other);
     constexpr BitSetArray &operator^=(const BitSetArray &other);
@@ -717,6 +701,7 @@ class BitSetArray final
 
     std::array<BaseBitSet, kArraySize> mBaseBitSetArray;
 };
+static_assert(std::is_trivially_copyable<BitSetArray<32>>(), "must be memcpy-able");
 
 template <std::size_t N>
 constexpr BitSetArray<N>::BitSetArray()
@@ -763,15 +748,6 @@ constexpr BitSetArray<N>::BitSetArray(std::initializer_list<param_type> init)
     }
 }
 
-template <size_t N>
-constexpr BitSetArray<N>::BitSetArray(const BitSetArray<N> &other)
-{
-    for (std::size_t index = 0; index < kArraySize; index++)
-    {
-        mBaseBitSetArray[index] = other.mBaseBitSetArray[index];
-    }
-}
-
 template <size_t N>
 BitSetArray<N>::Iterator::Iterator(const BitSetArray<N> &bitSetArray, std::size_t index)
     : mIndex(index),
@@ -832,16 +808,6 @@ std::size_t BitSetArray<N>::Iterator::operator*() const
     return (mIndex * priv::kDefaultBitSetSize) + *mCurrentIterator;
 }
 
-template <std::size_t N>
-constexpr BitSetArray<N> &BitSetArray<N>::operator=(const BitSetArray<N> &other)
-{
-    for (std::size_t index = 0; index < kArraySize; index++)
-    {
-        mBaseBitSetArray[index] = other.mBaseBitSetArray[index];
-    }
-    return *this;
-}
-
 template <std::size_t N>
 constexpr BitSetArray<N> &BitSetArray<N>::operator&=(const BitSetArray<N> &other)
 {

src/libANGLE/ProgramExecutable.cpp

@@ -300,9 +300,6 @@ void LoadSamplerBindings(BinaryInputStream *stream,
 }
 }  // anonymous namespace
 
-ProgramExecutable::PODStruct::PODStruct()                       = default;
-ProgramExecutable::PODStruct::PODStruct(const PODStruct &other) = default;
-
 ProgramExecutable::ProgramExecutable() : mActiveSamplerRefCounts{}
 {
     memset(&mPODStruct, 0, sizeof(mPODStruct));

src/libANGLE/ProgramExecutable.h

@@ -110,7 +110,7 @@ struct ProgramInput
     std::string mappedName;
 
     // The struct bellow must only contain data of basic type so that entire struct can memcpy-able.
-    struct
+    struct PODStruct
     {
         uint16_t type;  // GLenum
         uint16_t arraySizeProduct;
@@ -135,6 +135,7 @@ struct ProgramInput
 
         uint32_t id;
     } basicDataTypeStruct;
+    static_assert(std::is_trivially_copyable<PODStruct>(), "must be memcpy-able");
 };
 ANGLE_DISABLE_STRUCT_PADDING_WARNINGS
 
@@ -534,9 +535,6 @@ class ProgramExecutable final : public angle::Subject
     // This struct must only contains basic data types so that entire struct can be memcpy.
     struct PODStruct
     {
-        PODStruct();
-        PODStruct(const PODStruct &other);
-
         ShaderBitSet linkedShaderStages;
         angle::BitSet<MAX_VERTEX_ATTRIBS> activeAttribLocationsMask;
         unsigned int maxActiveAttribLocation;
@@ -581,11 +579,8 @@ class ProgramExecutable final : public angle::Subject
         UniformBlockBindingMask activeUniformBlockBindings;
 
         ShaderMap<int> linkedShaderVersions;
-        static_assert(std::is_trivially_copyable<ShaderMap<int>>(),
-                      "ShaderMap<int> should be trivial copyable so that we can memcpy");
     } mPODStruct;
-    static_assert(std::is_standard_layout<PODStruct>(),
-                  "PODStruct must be a standard layout struct so that we can memcpy");
+    static_assert(std::is_trivially_copyable<PODStruct>(), "must be memcpy-able");
 
     // Cached mask of active samplers and sampler types.
     ActiveTextureMask mActiveSamplersMask;

src/libANGLE/Uniform.cpp

@@ -44,7 +44,7 @@ void ActiveVariable::unionReferencesWith(const LinkedUniform &other)
     }
 }
 
-LinkedUniform::LinkedUniform() {}
+LinkedUniform::LinkedUniform() = default;
 
 LinkedUniform::LinkedUniform(GLenum typeIn,
                              GLenum precisionIn,
@@ -78,11 +78,6 @@ LinkedUniform::LinkedUniform(GLenum typeIn,
     }
 }
 
-LinkedUniform::LinkedUniform(const LinkedUniform &other)
-{
-    memcpy(this, &other, sizeof(LinkedUniform));
-}
-
 LinkedUniform::LinkedUniform(const UsedUniform &usedUniform)
 {
     ASSERT(!usedUniform.isArrayOfArrays());
@@ -118,8 +113,6 @@ LinkedUniform::LinkedUniform(const UsedUniform &usedUniform)
     ASSERT(!usedUniform.isArray() || arraySize == usedUniform.getArraySizeProduct());
 }
 
-LinkedUniform::~LinkedUniform() {}
-
 BufferVariable::BufferVariable()
     : bufferIndex(-1), blockInfo(sh::kDefaultBlockMemberInfo), topLevelArraySize(-1)
 {}

src/libANGLE/Uniform.h

@@ -74,9 +74,7 @@ struct LinkedUniform
                   const int locationIn,
                   const int bufferIndexIn,
                   const sh::BlockMemberInfo &blockInfoIn);
-    LinkedUniform(const LinkedUniform &other);
     LinkedUniform(const UsedUniform &usedUniform);
-    ~LinkedUniform();
 
     bool isSampler() const { return GetUniformTypeInfo(type).isSampler; }
     bool isImage() const { return GetUniformTypeInfo(type).isImageType; }
@@ -164,6 +162,7 @@ struct LinkedUniform
     // sh::ShaderVariable::id or sh::InterfaceBlock::id
     ShaderMap<uint32_t> mIds;
 };
+static_assert(std::is_trivially_copyable<LinkedUniform>(), "must be memcpy-able");
 ANGLE_DISABLE_STRUCT_PADDING_WARNINGS
 
 struct BufferVariable : public sh::ShaderVariable

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -8029,7 +8029,6 @@ angle::Result ContextVk::switchToReadOnlyDepthStencilMode(gl::Texture *texture,
         return angle::Result::Continue;
     }
 
-    vk::RenderPassUsageFlags oldUsageFlags = mDepthStencilAttachmentFlags;
     if (isStencilTexture)
     {
         if (mState.isStencilWriteEnabled())