0

I have the following c++ which previously worked on windows using a different compiler and on the w3schools compiler in browser.

#include "library.h"
#include <iostream>

void cluster(double* X, int Ntrj, int Nd, int Ngrid){
    int strides[Nd], coords[Nd];
    int* pstrides = &(strides[Nd-1]);
    int* pcoords = &(coords[0]);

    strides[Nd-1] = 1;

    //std::cout << *pstrides << std::endl;

    for (int i = Nd-2; i >=0 ; i--){
        *(--pstrides) = *pstrides*Ngrid;
        //std::cout << *pstrides << std::endl;
    }


}

Compiling with g++ on macos I get the following error,

warning: unsequenced modification and access to 'pstrides' [-Wunsequenced]
        *(--pstrides) = *pstrides*Ngrid;
          ^              ~~~~~~~~

I do not understand why this compiles in some cases and why in others it doesn't. I am c++ noob so any help would be appreciated. Thanks.

Kyle
  • 115
  • 7
  • 2
    `I do not understand why this compiles in some cases and why in others it doesn't` It _does_ compile, that's a warning, not an error. Its undefined behavior though. – tkausl Jul 23 '22 at 12:17
  • 1
    2 problems: (1) `int strides[Nd], coords[Nd];` is only supported by compiler extension and all modern compilers running in conformance mode will notify you about this - live - https://godbolt.org/z/7j1P6Tjjs (2) in `*(--pstrides) = *pstrides*Ngrid;` modifying and reading the same variable (`pstrides`) in an expression is Undefined Behaviour (before C++ 20 [Order of evaluation point (20)](https://en.cppreference.com/w/cpp/language/eval_order)). – Richard Critten Jul 23 '22 at 12:17
  • The compiler is just warning you: It's unspecified, if in `*(--pstrides) = *pstrides*Ngrid;` `--pstrides` or `*pstrides` gets evaluated first. You should be storing the rhs in a variable and assign this value to `*(--pstrides)`. On the other hand you could simply use a value to store the current value. `int value = 1; for (auto begin = std::begin(strides), pos = std::end(strides); pos != begin; value *= Ngrid) { --pos; *pos = value; }` – fabian Jul 23 '22 at 12:24
  • @GoswinvonBrederlow your code doesn't even compile. I assume the intention was to fill the array with `Ngrid^(n), Ngrid^(n-1), ..., Ngrid^0` which my code does. – fabian Jul 23 '22 at 12:43
  • @fabian the use of begin/ end is not possible? The size of strides is not known at compile time. This function is called externally in Python. – Kyle Jul 25 '22 at 14:51
  • @Kyle You shouldn't be using VLAs anyways; they aren't part of the C++ standard. Use `std::vector` instead. The code snippet works with arbitrary types. You can replace `std::begin(strides)` and `std::end(strides)` with other values such as the begin/end iterators of a vector or even pointers (`auto begin = strides, pos = strides + Nd`) – fabian Jul 25 '22 at 16:26

0 Answers0