search

Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

range-loop-analysis checks for trivial copyability, rather than trivial copy-constructibility #46980

Open Quuxplusone opened 4 years ago

Quuxplusone commented 4 years ago
Bugzilla Link PR48011
Status NEW
Importance P normal
Reported by Jake Arkinstall (jake.arkinstall@gmail.com)
Reported on 2020-10-29 09:40:52 -0700
Last modified on 2020-12-09 11:35:03 -0800
Version 11.0
Hardware PC Linux
CC dblaikie@gmail.com, dcoughlin@apple.com, feqin1023@gmail.com, jason.e.cobb@gmail.com, llvm-bugs@lists.llvm.org, neeilans@live.com, paul_robinson@playstation.sony.com, richard-llvm@metafoo.co.uk, thomasw@crytek.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

On the Cpplang slack, user Wolle posted a curious warning that came from range-loop-analysis.

Here's a minimal example: https://godbolt.org/z/1cxs43. Note that without the copy assignment operator overload, the warning is not present.

The range-based for loop performs a copy construction, so that is what should be checked for.

At https://clang.llvm.org/doxygen/SemaStmt_8cpp_source.html#l02824, the check is on whether the type is trivially copyable, not whether it is trivially copy constructible, which is a weaker requirement (but the relevant one in this case). I am not intimately familiar with clang sourcecode, but I believe this to be the cause of the warning.

Quuxplusone commented 4 years ago

Static analyzer does not participate at all in this warning. It is a normal frontend warning. To Frontend component.

Quuxplusone commented 4 years ago 
I seem to remember running into this with respect to a container containing
std::pair elements, but I can't find the details now.  I thought I'd filed
a bug but apparently not.
Quuxplusone commented 3 years ago 
A not very long time ago, I have been worked with clang frontend.
should I try this bugfix?
Quuxplusone commented 3 years ago 
(In reply to david fuqiang fan from comment #3)
> A not very long time ago, I have been worked with clang frontend.
> should I try this bugfix?

I think it should be a relatively simple thing to fix, yes. You're quite
welcome to have a go at it.
Quuxplusone commented 3 years ago 
(In reply to David Blaikie from comment #4)
> (In reply to david fuqiang fan from comment #3)
> > A not very long time ago, I have been worked with clang frontend.
> > should I try this bugfix?
>
> I think it should be a relatively simple thing to fix, yes. You're quite
> welcome to have a go at it.

Ok, I am working.
Quuxplusone commented 3 years ago 
(In reply to Jake Arkinstall from comment #0)
> On the Cpplang slack, user Wolle posted a curious warning that came from
> range-loop-analysis.
>
> Here's a minimal example: https://godbolt.org/z/1cxs43. Note that without
> the copy assignment operator overload, the warning is not present.
>
> The range-based for loop performs a copy construction, so that is what
> should be checked for.
>
> At https://clang.llvm.org/doxygen/SemaStmt_8cpp_source.html#l02824, the
> check is on whether the type is trivially copyable, not whether it is
> trivially copy constructible, which is a weaker requirement (but the
> relevant one in this case). I am not intimately familiar with clang
> sourcecode, but I believe this to be the cause of the warning.

I can not find your name as a reviewer.

https://reviews.llvm.org/D92956