ymandel added a comment. In D150653#4346025 <https://reviews.llvm.org/D150653#4346025>, @mboehme wrote:
> In D150653#4345771 <https://reviews.llvm.org/D150653#4345771>, @ymandel wrote: > >> I'm a little confused by one of the points in the description: >> >>> `setValueStrict()`: The RFC proposes that this should always create the >>> same StorageLocation for a given Value, but, in fact, the transfer >>> functions that exist today don't guarantee this; almost all transfer >>> functions unconditionally create a new `StorageLocation` when associating >>> an expression with a `Value`. >> >> Since values are immutable, we (intentionally) share them between different >> storage locations. So, it makes sense that a single value could be >> associated with multiple storage locations. Now, there's a bug in that >> values aren't really immutable since you can update the fields, so we have a >> false sharing bug in the framework, but maybe that only applies to glvalues? > > Yes, values are and should definitely be shared between different storage > locations. > > The comment in the RFC is specifically in the context of the "fake" storage > locations that we create for prvalues (since `setValueStrict()` only deals > with prvalues). I call these storage locations "fake" because a prvalue > doesn't actually have a storage location. During the review of the RFC, one > of the reviewers asked whether we should guarantee that a given `Value`, when > associated with a prvalue `Expr`, should always use the same "fake" storage > location, instead of creating a new storage location every time. The idea was > that this might be required for convergence. This argument made sense to me > at the time, but having looked at what the existing code does, almost every > place that associates a `Value` with a prvalue expression unconditionally > create a new "fake" storage location. So this implies to me that creating > durable fake storage locations isn't necessary. > > Does this make sense? Yes, makes sense. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150653/new/ https://reviews.llvm.org/D150653 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
