1,119 karma · joined January 8, 2014
I don't think that this is true. `std::deque` has guaranteed O(1) complexity for insertions or deletions at the beginning or end of the container. It couldn't provide this if it was moving every value in the queue.
> How do we alter the stackSum when the queue moves?
You never quite answer this question. How are `sumOut` and `sumIn` related to `stackSum`? This is available in the code but I feel like there should at least be a one-line summary available without having to click through to the code.
> sumOut is what we need to add to the stack.
This is supposed to be `sumIn`?
The examples that you provide use a radius of 2, thus having a `sizeOfStack` of (radius+1)*2 = 9 and requiring a division by 9 for each pixel. Have you considered using a radius of 1 or a radius of 3 instead? This would require division by (1+1)*2 = 4 or (3+1)*2 = 16 respectively which could then be done with a bitshift instead of a division. You'd have to template the code on `radius` instead of passing it in as a runtime parameter so that the compiler could lower the divisions to bitshifts.
1. The implementation of `FromBytes` works properly only if the base case returns 0 for the empty array. So you already have, for free, `FromBytes([]) == 0`. You might as well use it.
2. `LemmaFromToBytes` currently requires the somewhat awkward and complicated `requires |bytes| > 0 && (|bytes| == 1 || bytes[0] != 0)`. We need to restrict this to only allowing non-empty arrays. And then we need to restrict it to not allowing leading zeros on arrays, except if the array is of length 1 and then we do allow a leading zero. If we represent 0 as `[]` instead of `[0]` then this all simplifies down to something like `requires |bytes| == 0 || bytes[0] != 0`. We allow arrays of any size and we never allow leading zeros on any array, which is easier to express.
3. Currently `ToBytes` has an `if-else` at the end and requires repeating the code `[byte]` in both branches. That's obviously not a lot of repetition but wouldn't it be nice to eliminate that? If we change the beginning to `if v == 0 then []` then we can replace the `if-else` at the end with just `ToBytes(w) + [byte]`. And we can get rid of the `ensures |r| > 0`. Then the structure of `ToBytes` and `FromBytes` more closely mirror each other, with the base cases being "the same".
This is usually what you want so that memory addresses don't change unnecessarily from one run to the next. But if you're debugging a problem that only shows up when ASLR is in effect then you can turn it back on. https://visualgdb.com/gdbreference/commands/set_disable-rand...
std::unique_ptr<char, free_deleter> line;
size_t len = 0;
while ((read = getdelim(std::inout_ptr(line), &len, delim, fp)) != -1) {
// ...
}
If the getdelim call ends up calling realloc and succeeding then we want to forget about the old pointer and not call `free` on it. We want to remember the new pointer and call `free` on that when we're done with the loop. That's what std::inout_ptr gets for us. It calls release() before the getdelim call and then calls reset() with the new value after the getdelim call.If there is an I/O error or a realloc failure at some point in the file then `line` is left unmodified, pointing to the existing buffer. We still need to free that so keeping it under the control of the smart pointer is the right thing to do.
This seems like a perfect fit for std::inout_ptr.
The situation asprintf() is unfortunate. If it had been designed to set the out parameter to NULL on error or just to guarantee that it was unmodified on error then it would work with std::out_ptr. Unfortunately they decided that the out pointer would be instead be undefined on error so it is not safe to use with std::out_ptr.
void frob(std::string_view sv);
std::string a = "a";
std::string b = "b";
frob(a + b);
This is allowed today and does not have any problems that I'm aware of. You could work around this by changing the call to: frob(std::string_view(a + b));
But that feels cumbersome. I think the goal of std::string_view was to be useful as a parameter for a function so that you could pass in anything remotely string-like and it would implicitly convert and do what you meant. Requiring explicit conversions at the call sites would go against that goal.Another thing that was desired was the ability to take an existing function that takes a `const std::string&` and change it to instead take a `std::string_view` and not have to update any of the callers. If some callers that worked with the old function are going to need an update to work with the new function then it gets a lot harder to change the function. Or maybe impossible if you don't control all of the callers.
I don't see how you can get both the ease-of-use and the safety short of reinventing something like Rust's lifetimes.
OK, I'll give you that one, but that's a rather unusual case. It has complicated semantics. It is unlikely to interoperate well with any smart pointer without careful, manual work.
It doesn't take a `T**` so it doesn't seem relevant to discussions of std::out_ptr though? You won't be able to use std::out_ptr with realloc so there's no chance of making a mistake with it.
> Generally when you have an in-out parameter, it's doing more than just releasing resources
I'm still confused. What function do you have that is both releasing resources and also doing other work which might fail? Even something like `fclose` which needs to flush buffers and then release resources will always release the resources, even if the flush fails.
I'm more familiar with APIs that have:
- One (or more) functions to explicitly allocate the object
- Many functions that do work with that object and that might fail but that will never deallocate the object
- One function to explicitly deallocate the object
For example, SQLite has sqlite3_open to create a new database object, sqlite3_prepare, sqlite3_exec, etc. to operate on the database object, and sqlite3_close to deallocate the database object.
Say that you have some `Person` class with a `get_name()` method that returns the person's name. Then maybe you write some code like this to use it:
Person person;
// ...
std::string_view name(person.get_name());
// Use `name` here
Is this code safe? Well it depends on the implementation of `Person::get_name`. Maybe it looks like this: std::string& Person::get_name() {
return m_name;
}
Then the code should be OK because the call will return a reference to a member of `person` and `person` will outlive `name`.But what if instead the implementation looks like this:
std::string Person::get_name() {
return m_first_name + " " + m_last_name;
}
Now you're in trouble. A temporary string will be created and the string_view will refer to that string. That temporary gets destroyed at the end of the statement and then `name` is dangling. By the time you get to use `name` it is already broken.It is very easy to make this kind of mistake. Much easier than with pointers IMO since you usually have to explicitly take a pointer to something and you can't directly get a pointer to a temporary (`std::string* p = &person.get_name()` is an error if `Person::get_name` returns `std::string`).
By contrast, getting a const reference to a temporary and then constructing a string_view out of it is not an error or even a warning. It is all done implicitly so there is no indication in the code that it is even happening. The only difference between code that is correct and code that is incorrect is a single ampersand far away in some header file.
This type of mistake probably won't get caught in code review. How often does a person really go track down and examine a header file when reviewing a pull request? Probably they're only going to look at the actual files that are being changed.
Even worse, say that `Person::get_name` is using the first implementation so the code is correct and working. But later on somebody does a refactor such `Person::get_name` uses the second implementation. Now they've broken things but they're unaware of this fact. They want to be responsible and find any problems that they might have introduced. The first step is to run a build and see if there are any complaints from the compiler. That is successful and the compiler produces no warnings.
Encouraged by this they then run all the tests. And all the tests pass! Reading memory just after it has been freed will often give you back the last thing that was stored there so the tests see the values that they expect. So with a successful build, a successful run of all the tests, and no problems spotted in code review, they confidently merge the change. And now you've got a use-after-free in your code that will eventually cause a problem.
Yes, using Address Sanitizer will catch this. But it is still a pretty easy way to introduce a bug into your program. Code that does not look at all suspicious can be completely broken.
void c_api_that_returns_via_an_out_parameter(T** ret) {
*ret = new T();
}
T* p;
c_api_that_returns_via_an_out_parameter(&p);
Then you might try to change this to use a smart pointer as: std::unique_ptr<T> p;
c_api_that_returns_via_an_out_parameter(&p);
But this won't compile because you can't take the address of a `std::unique_ptr<T>` and then pass it where a `T**` is expected. So instead you need to do something like this: T* tmp = 0;
c_api_that_returns_via_an_out_parameter(&tmp);
std::unique_ptr<T> p(tmp);
That can be a bit of a pain. These new functions serve as adapters to make this easier. They take in a `std::unique_ptr<T>` and provide a `T*` that you can pass to the C API: std::unique_ptr<T> p;
c_api_that_returns_via_an_out_parameter(std::out_ptr(p));
No temporary needed.If your C API only ever passes and returns `T*` and never uses `T**` then I think that you don't need these new functions and you can continue to do things the current way. For instance, if you have a C API that allocates a new object and then returns a pointer to it then you can take this return value and immediately store it into your smart pointer:
std::unique_ptr<T> p(c_api_that_returns_a_pointer());
And if you have a C API that takes a `T**` and then deallocates it: c_api_that_deallocates(p.release());
Although really your smart pointer should be doing this for you so you'd never need to do it explicitly. If you're ever manually calling `.release()` then that indicates a place where the smart pointer abstraction has broken down. You want to minimize or eliminate that. Note that in the examples using the new functions there are no explicit calls to `.release()` anywhere and this is a good thing.I can't recall ever seeing a C API that conditionally deallocates the pointer passed in. Do you have an example? Most C APIs that I'm familiar with have one function that allocates a new object and a separate function that unconditionally deallocates it, acting analogously to `malloc` and `free`.
$ apropos search
...
grep (1p) - search a file for a pattern
...I don't think that can be correct. This would allow FTL communication of information. I could put the filter in the path of the second photon when I wanted to send a 1 and leave it out of the path of the second photon when I wanted to send a 0. In the 1 case the other side would sometimes see the photon pass both filters. In the 0 case the other side would never see the photon pass both filters. Combine this with a little error correction and you have a channel for transmitting arbitrary information faster than light which would violate the no-communication theorem. https://en.wikipedia.org/wiki/No-communication_theorem
The MinutePhysics video you link describes a different setup with only one filter in the path of each photon. This gives you some information about the probability that other photon got past its filter but it doesn't let you transmit anything.
Sometimes even -fno-omit-frame-pointer won't help, like if the stack is being unwound through a system library that was built without frame pointers. In that case you can switch to the slow unwinder. Set the environment variable `ASAN_OPTIONS=fast_unwind_on_malloc=0` when running your program. But note that this will make most programs run significantly slower so you probably want to use it only when you really need it and not as the default setting for all runs.