In this patch, move constructor/assignment operator is defined for LoopAccessInfo. PredicatedScalarEvolution PSE member is also changed to be unique_ptr.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Minor drop by comment inline.
include/llvm/Analysis/LoopAccessAnalysis.h | ||
---|---|---|
530 ↗ | (On Diff #61644) | This looks strange -- when will this check be true? |
include/llvm/Analysis/LoopAccessAnalysis.h | ||
---|---|---|
530 ↗ | (On Diff #61644) | Self assignment. |
include/llvm/Analysis/LoopAccessAnalysis.h | ||
---|---|---|
530 ↗ | (On Diff #61644) | I mean -- do we need to support x = std::move(x); ? Or is it better to assert out in such cases? Are there ways such an assignment can come up organically? |
include/llvm/Analysis/LoopAccessAnalysis.h | ||
---|---|---|
530 ↗ | (On Diff #61644) | not likely. I can change it to assert as you suggested. |
include/llvm/Analysis/LoopAccessAnalysis.h | ||
---|---|---|
518–527 ↗ | (On Diff #61807) | How are these different from the default move ctor/assignment operator? |
633 ↗ | (On Diff #61807) | Hmm, const member returning a non-const reference? |
634 ↗ | (On Diff #61807) | Can we make this private now? |
770–780 ↗ | (On Diff #61807) | Does this belong to this patch? |
include/llvm/Analysis/LoopAccessAnalysis.h | ||
---|---|---|
518–527 ↗ | (On Diff #61807) | Sean mentioned that some compiler (MVS) does not generate default move ctor properly. |
include/llvm/Analysis/LoopAccessAnalysis.h | ||
---|---|---|
518–527 ↗ | (On Diff #61807) | So they're not different? A comment explaining this would be good. |
Quick comment to avoid a buildbot issue.
include/llvm/Analysis/LoopAccessAnalysis.h | ||
---|---|---|
518 ↗ | (On Diff #62397) | The issue with MSVC is actually that it can't synthesize this, so = default doesn't help unfortunately. You have to manually write out the entire move ctor like you had it. Just make sure to add a comment " // Hack for MSVC 2013 which seems like it can't synthesize this." or similar. |