Make locations_array constexpr and move curr_statement__ to function scope

[SteveBronder]

Summary

This does three things

1. Makes locations_array__ into a static constexpr std::array<const char*, {size_of_error_list}>
2. Moves current_statement__ to be created inside of local function scopes.
3. Adds spaces to the templates for the _impl functions so they are a little easier to read.

(1) is just a nice optimization. That array is only read from and we can deduce the size in the compiler so we just make it a static constexpr array of const char* so the compiler knows it's size and static so that it knows that global can only be accessed inside this file.

(2) is related to #897 which let's a Stan program run with multiple chains in one program. Having current_statement__ as a global variable means that when we call log prob in that etc. in that PR we hit a bunch of weird race conditions since each thread can be at a different point in each function (or running seperate functions that update current_statement__. This also produces false sharing in loops. Like say a user wrote

for (i in 1:N) {
  lp[i] = log_mix(x, xx[i], yy[i]);
  lp2[i] = log_mix(x2, xx2[i], yy2[i]);
}

In the c++ that becomes

// pseudocode
for (int sym1__ = 1; sym1__ <= N;  ++sym1__) {
  current_statement__ = 5;
  lp[i] = log_mix(x, xx[i], yy[i]);
  current_statement__ = 6;
  lp2[i] = log_mix(x2, xx2[i], yy2[i]);
}

Since we are writing to a global if current_statement__, if the scalars used in the function and current_statement__ are used on the same cache line then the cpu marks that cache line as invalid and forces the entire cache line to be re-read back in from memory. By making it defined locally each thread has their own version of curr_statement__ and so there is no false sharing

So we just make current statement locally above the try block for each function that uses it. Thinking now I need to see how this will work with user defined functions. I think I sorted that out by just making it locally in the udf

(3) is just a nice thing to be able to read the generated C++

Release notes

Make locations_array constexpr and move curr_statement__ to function scope

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

Copyright Holder: Steve Bronder

[rok-cesnovar]

1 and 2 look great and make sense to me. Are you trying to limit the number of characters in a line for 3? Or are you interested in actual spaces?

[rok-cesnovar]

I am not completely sold on this @ \x20 thing. What you are trying to do is just limit the number of characters in a line right? If so, we can do this with less manual intervention.

Also dont really like the space before the comma in (see jacobian)

template <bool propto__, bool jacobian__ , typename VecR, typename V

[SteveBronder]

I just thought it was nice looking to have an indentation there but I removed them

[rok-cesnovar]

Oh indentation, I get it. Not an expert on this printing/formatter stuff, but I think there is a way to do that without the use of hex. Will give it a shot, else just merge if that is ok with you?

[SteveBronder]

Yeah it's fine as is

[rok-cesnovar]

It seems we will have to revert a part of this. I am seeing

examples/test/test.hpp:1029:41: warning: ISO C++ forbids converting a string constant to ‘char*’ [-Wwrite-strings]
 1029 |     static constexpr char* function__ = "test_model_namespace::test_model";

using g++ (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0

[SteveBronder]

Ben just merged a PR that should fix this

[rok-cesnovar]

Indeed. That was fast :)