Tpetra::CrsGraph, CrsMatrix: Constructor that takes an array of max row counts, takes it by ArrayRCP, but doesn't actually keep the reference
Created by: mhoemmen
@trilinos/tpetra @vbrunini
Both Tpetra::CrsGraph
and Tpetra::CrsMatrix
have constructors that take an array of the maximum number of entries in each row. The constructors take that array by Teuchos::ArrayRCP<const size_t>
. Idiomatically, taking an ArrayRCP of const suggests that the class keeps a reference to the array, so that users shouldn't modify it. This is not what happens. Tpetra actually deep-copies the array.
We should thus change the constructors so they take either a raw array, or Teuchos::ArrayView<const size_t>
. Once we have C++20, we can use std::span
, which exactly expresses the desired idiom in native C++.
I found this by looking through Chaparral (@vbrunini of course has already fixed it to use StaticProfile, because he is just effective like that ;-) ). I saw that it was using a nonowning ArrayRCP there, and got worried, but then discovered that it didn't matter, since CrsGraph (and thus CrsMatrix) just deep-copies it.