Avoiding “Overlapping Access” errors

I'm writing a pure-Swift GeoTIFF parser. Part of that involves creating a BinaryReader struct that maintains an index into a Data, and allows seeking and reading different types.

Something I commonly do is save the current index, seek to some other part of the file, read some data, then seek to the saved index (note that read() accesses self.reader in a loop):

let saveIdx = self.reader.idx
self.reader.seek(to: de.offset)
ifd.stripByteCounts = try read(count: de.count, ofType: de.type)
self.reader.seek(to: saveIdx)

I thought this boilerplate could be cleaned up with a method that takes a closure, like this:

try self.reader.at(de.offset)
{
    ifd.stripByteCounts = try read(count: de.count, ofType: de.type)
}

where at(_,op:) looks like:

mutating
func
at(_ inOffset: UInt64, op inOp: () throws -> ())
    throws
{
    let saveIdx = self.idx
    seek(to: inOffset)
    try inOp()
    seek(to: saveIdx)
}

Unfortunately, when I call at(_,:op:), I get

Overlapping accesses to 'self.reader', but modification requires exclusive access; consider copying to a local variable.

If BinaryReader is a class and not a struct, this error goes away. I’m not sure semantically which makes more sense. Perhaps class, because I find that all the methods are mutating (despite coding in Swift for many years now, I’m still not sure of these sorts of things). But even as a struct, the code seems valid, so is there a way to tell the compiler it’ll be okay?

Not really. The call to read inside inOp is access the value of reader while you're in the middle of a mutating function (at()) that's changing the internal state of reader. That's not safe.

Why is at() a function in BinaryReader? It seems more natural to make it a sibling function of read:

func at(_ inOffset: UInt64, op inOp: () throws -> ())
    throws
{
    let saveIdx = self.reader.idx
    self.reader.seek(to: inOffset)
    try inOp()
    self.reader.seek(to: saveIdx)
}

I think this would solve your access problem, too.

1 Like

I disagree that it seems more natural to pull it out of BinaryReader. If some other class wants to use BinaryReader it would likely have to implement the same code. But it's perfectly safe, or rather, it's no less safe than calling the code inside at() directly. Unless I’m missing something? How is it unsafe? Changing it to class, the code is exactly the same, but the mutation-within-a-mutation is now safe?

Or does it just make more sense for BinaryReader to be a class?

This blog post contains the answers to your questions:

1 Like

Okay, that clarifies a distinction between struct and class, thank you. The referenced post directly addresses my use, although it’s still frustrating that I can’t express what I want.

From the client’s perspective, should BinaryReader be a struct or a class? I can make it a struct by passing the reader into the closure as an inout parameter. Doing so encourages me to make my read method an extension of BinaryReader, which make sense, until you get to readString(), which depends on a property of my class. That can be passed in as an argument, but either way it’s messier.

I’m not happy with any solution of these solutions, except maybe making it a class.

The easiest solution that maintains value semantics, and one used in NIO pervasively, is to pass self inout to the closure:

mutating
func
at(_ inOffset: UInt64, op inOp: (inout Self) throws -> ())
    throws
{
    let saveIdx = self.idx
    seek(to: inOffset)
    try inOp(&self)
    seek(to: saveIdx)
}

This communicates to the compiler what you’re trying to do. It’s moderately awkward because you have to remember to not to use the outer self within the block, but it has the effect of clearly communicating the ownership semantics of self at all times.

2 Likes

Thanks for that confirmation. The only drawback to that is that my read method above references both self.reader and a member of the calling struct, meaning I have to pass one or the other to the method. It’s not the end of the world, and in fact that’s what I did because I wanted my two read methods to be extensions on BinaryReader rather than members of the calling struct. But it just wasn’t as nice.

I still don't know just how desirable value semantics are for this type. The thing that's constantly mutated as it's used is the internal index into the data source. The intent of the at() method is to provide nestable push/pop of the current index, so the parsing can jump around through it. It does that on the stack through recursion, but an alternative might be to make a copy of BinaryReader. Each copy would copy the current index and the Data it holds, but that Data is never mutated, and thus should not be copied in practice. Oh, nevermind, I just realized this won’t help, because once I make a copy of BinaryReader I still have to modify the index. Not sure value semantics provides a benefit, but I’d sure like to know if it does!

My first-order heuristic for design is that I attempt to build with value semantics first, and only abandon it if it's clear that what I'm modelling has meaningfully got referential behaviour. Value semantics make many programming use-cases substantially easier and make it much more straightforward to verify the correctness of code.

I will add that there are some things that are never likely to be meaningfully value-semantic: a file descriptor, for example, or a handle, are always going to be referential. But for many other things, particularly data structures that do purely in-memory computation, value semantics can be a huge win.

5 Likes