[PATCH] D153491: [dataflow] Avoid copying environment

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 23 15:51:25 PDT 2023


sammccall added a comment.

In D153491#4445051 <https://reviews.llvm.org/D153491#4445051>, @ymandel wrote:

> In D153491#4443704 <https://reviews.llvm.org/D153491#4443704>, @xazax.hun wrote:
>
>> This sounds extremely error-prone to me. In case copying the analysis state has side effects like this, I would argue we want such operations to be really explicit. What do you think?
>
> Can you expand on this concern? Are you referring to the removal of the unintended copy (ie. this patch), or raising a concern about the how the underlying system handles copies altogether?

FWIW I would prefer if we can call this operation clone() or fork() or something rather than have it be the copy constructor.

It is a kind of copy but between it being quite expensive, this effect on the SAT system, and intentional copies being quite an important and deliberate thing, making it a bit verbose seems justified.

I'll send a patch for this...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153491/new/

https://reviews.llvm.org/D153491



More information about the cfe-commits mailing list