ojhunt wrote:

> Thanks for the patch !
> 
> > We could restrict this to just those targeting this inside a constructor, 
> > but I've also found cases where constructors memset/bzero subobjects?
> 
> Having a look at https://godbolt.org/z/7KThvxKx5 I'd feel better if we could 
> capture the idiom in `ValidFoo` while still warning about `InvalidFoo`.
> 
> The scope of your patch is much larger, and it would miss several cases where 
> it should legitimately warn.

Why should the `InvalidFoo` case warn? Bar is trivially initializable, and the 
reason InvalidFoo is not is solely the constructor that uses memset/bzero

That is the _exact_ case that I am saying is present in idiomatic code, that is 
being triggered in a lot of different code bases. This code is zero 
initializing a struct, the members are trivially initializable. They aren't 
being copied, so why would non-trivial copyability matter in this warning?

This also gets triggered outside of constructors

```cpp
struct Struct {
  Struct() = default;
  Struct(const Struct&);
};
...
Struct s;
memset(&s, 0, sizeof(s))
```

This is _idiomatic_ and wide spread: the alternative is having to manually 
initialize every field, and if new fields are added you have to make sure all 
sites are updated. Initializing each field also doesn't clear the padding bits, 
etc.

I think the issue here is that you haven't explained _why_ the copyability of a 
type has any impact on the semantics of an initialization function?

https://github.com/llvm/llvm-project/pull/170577
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to