Skip to content

Commit 2b8d411

Browse files
nathan-russellnathan-russell
committed
Fix upper_tri and lower_tri; add unit tests (closes #641)
1 parent 5a99a86 commit 2b8d411

File tree

6 files changed

+296
-203
lines changed

6 files changed

+296
-203
lines changed

‎ChangeLog

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
2017-01-31 Nathan Russell <[email protected]>
2+
3+
* inst/include/Rcpp/sugar/matrix/upper_tri.h: Inherit from MatrixBase
4+
and use correct comparators in get() to fix segfault
5+
* inst/include/Rcpp/sugar/matrix/lower_tri.h: Idem
6+
* inst/unitTests/cpp/sugar.cpp: Added unit tests for upper_tri and
7+
lower_tri
8+
* inst/unitTests/runit.sugar.R: Idem
9+
110
2017-01-23 James J Balamuta <[email protected]>
211

312
* inst/include/Rcpp/DataFrame.h: Corrected return type for column size.

‎inst/NEWS.Rd

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@
1010
\item Added new size attribute aliases for number of rows and columns in
1111
DataFrame (James Balamuta in \ghpr{638} addressing \ghit{630}).
1212
}
13+
\item Changes in Rcpp Sugar:
14+
\itemize{
15+
\item Fixed sugar functions \code{upper_tri()} and \code{lower_tri()}
16+
(Nathan Russell in \ghpr{642} addressing \ghit{641}).
17+
}
1318
}
1419
}
1520

‎inst/include/Rcpp/sugar/matrix/lower_tri.h

Lines changed: 29 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
// -*- mode: C++; c-indent-level: 4; c-basic-offset: 4; tab-width: 8 -*-
1+
// -*- mode: C++; c-indent-level: 4; c-basic-offset: 4; indent-tabs-mode: nil; -*-
22
//
33
// lower_tri.h: Rcpp R/C++ interface class library -- lower.tri
44
//
5-
// Copyright (C) 2010 - 2011 Dirk Eddelbuettel and Romain Francois
5+
// Copyright (C) 2010 - 2017 Dirk Eddelbuettel and Romain Francois
6+
// Copyright (C) 2017 Dirk Eddelbuettel, Romain Francois, and Nathan Russell
67
//
78
// This file is part of Rcpp.
89
//
@@ -22,60 +23,47 @@
2223
#ifndef Rcpp__sugar__lower_tri_h
2324
#define Rcpp__sugar__lower_tri_h
2425

25-
namespace Rcpp{
26-
namespace sugar{
26+
namespace Rcpp {
27+
namespace sugar {
2728

28-
template <int RTYPE, bool LHS_NA, typename LHS_T>
29-
class LowerTri : public VectorBase<
30-
LGLSXP ,
31-
false ,
32-
LowerTri<RTYPE,LHS_NA,LHS_T>
33-
> {
29+
template <int RTYPE, bool NA, typename T>
30+
class LowerTri : public MatrixBase<LGLSXP, false, LowerTri<RTYPE, NA, T> > {
3431
public:
35-
typedef Rcpp::MatrixBase<RTYPE,LHS_NA,LHS_T> LHS_TYPE ;
32+
typedef Rcpp::MatrixBase<RTYPE, NA, T> MatBase;
3633

37-
LowerTri( const LHS_TYPE& lhs, bool diag) :
38-
nr( lhs.nrow() ), nc( lhs.ncol() ),
39-
getter( diag ? (&LowerTri::get_diag_true) : (&LowerTri::get_diag_false) ){}
34+
LowerTri(const T& lhs, bool diag)
35+
: nr(lhs.nrow()),
36+
nc(lhs.ncol()),
37+
getter(diag ? (&LowerTri::get_diag_true) : (&LowerTri::get_diag_false))
38+
{}
4039

41-
// inline int operator[]( int index ) const {
42-
// int i = Rcpp::internal::get_line( index, nr ) ;
43-
// int j = Rcpp::internal::get_column( index, nr, i ) ;
44-
// return get(i,j) ;
45-
// }
46-
inline int operator()( int i, int j ) const {
47-
return get(i,j) ;
48-
}
40+
inline int operator()(int i, int j) const { return get(i, j); }
4941

50-
inline R_xlen_t size() const { return static_cast<R_xlen_t>(nr) * nc ; }
42+
inline R_xlen_t size() const { return static_cast<R_xlen_t>(nr) * nc; }
5143
inline int nrow() const { return nr; }
5244
inline int ncol() const { return nc; }
5345

5446
private:
55-
int nr, nc ;
56-
typedef bool (LowerTri::*Method)(int,int) ;
47+
typedef bool (LowerTri::*Method)(int, int) const;
5748

58-
Method getter ;
59-
inline bool get_diag_true( int i, int j ){
60-
return i <= j ;
61-
}
62-
inline bool get_diag_false( int i, int j ){
63-
return i < j ;
64-
}
65-
inline bool get( int i, int j){
66-
return (this->*getter)(i, j ) ;
67-
}
49+
int nr, nc;
50+
Method getter;
6851

69-
} ;
52+
inline bool get_diag_true(int i, int j) const { return i >= j; }
53+
54+
inline bool get_diag_false(int i, int j) const { return i > j; }
55+
56+
inline bool get(int i, int j) const { return (this->*getter)(i, j); }
57+
};
7058

7159
} // sugar
7260

73-
template <int RTYPE, bool LHS_NA, typename LHS_T>
74-
inline sugar::LowerTri<RTYPE,LHS_NA,LHS_T>
75-
lower_tri( const Rcpp::MatrixBase<RTYPE,LHS_NA,LHS_T>& lhs, bool diag = false){
76-
return sugar::LowerTri<RTYPE,LHS_NA,LHS_T>( lhs, diag ) ;
61+
template <int RTYPE, bool NA, typename T>
62+
inline sugar::LowerTri<RTYPE, NA, T>
63+
lower_tri(const Rcpp::MatrixBase<RTYPE, NA, T>& lhs, bool diag = false) {
64+
return sugar::LowerTri<RTYPE, NA, T>(lhs, diag);
7765
}
7866

7967
} // Rcpp
8068

81-
#endif
69+
#endif // Rcpp__sugar__lower_tri_h

‎inst/include/Rcpp/sugar/matrix/upper_tri.h

Lines changed: 30 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
// -*- mode: C++; c-indent-level: 4; c-basic-offset: 4; tab-width: 8 -*-
1+
// -*- mode: C++; c-indent-level: 4; c-basic-offset: 4; indent-tabs-mode: nil; -*-
22
//
3-
// upper_tri.h: Rcpp R/C++ interface class library -- lower.tri
3+
// upper_tri.h: Rcpp R/C++ interface class library -- upper.tri
44
//
5-
// Copyright (C) 2010 - 2011 Dirk Eddelbuettel and Romain Francois
5+
// Copyright (C) 2010 - 2017 Dirk Eddelbuettel and Romain Francois
6+
// Copyright (C) 2017 Dirk Eddelbuettel, Romain Francois, and Nathan Russell
67
//
78
// This file is part of Rcpp.
89
//
@@ -22,55 +23,47 @@
2223
#ifndef Rcpp__sugar__upper_tri_h
2324
#define Rcpp__sugar__upper_tri_h
2425

25-
namespace Rcpp{
26-
namespace sugar{
26+
namespace Rcpp {
27+
namespace sugar {
2728

28-
template <int RTYPE, bool LHS_NA, typename LHS_T>
29-
class UpperTri : public VectorBase<
30-
LGLSXP ,
31-
false ,
32-
UpperTri<RTYPE,LHS_NA,LHS_T>
33-
> {
29+
template <int RTYPE, bool NA, typename T>
30+
class UpperTri : public MatrixBase<LGLSXP, false, UpperTri<RTYPE, NA, T> > {
3431
public:
35-
typedef Rcpp::MatrixBase<RTYPE,LHS_NA,LHS_T> LHS_TYPE ;
32+
typedef Rcpp::MatrixBase<RTYPE, NA, T> MatBase;
3633

37-
UpperTri( const LHS_TYPE& lhs, bool diag) :
38-
nr( lhs.nrow() ), nc( lhs.ncol() ),
39-
getter( diag ? (&UpperTri::get_diag_true) : (&UpperTri::get_diag_false) ){}
34+
UpperTri(const T& lhs, bool diag)
35+
: nr(lhs.nrow()),
36+
nc(lhs.ncol()),
37+
getter(diag ? (&UpperTri::get_diag_true) : (&UpperTri::get_diag_false))
38+
{}
4039

41-
inline int operator()( int i, int j ) const {
42-
return get(i,j) ;
43-
}
40+
inline int operator()(int i, int j) const { return get(i, j); }
4441

45-
inline R_xlen_t size() const { return static_cast<R_xlen_t>(nr) * nc ; }
42+
inline R_xlen_t size() const { return static_cast<R_xlen_t>(nr) * nc; }
4643
inline int nrow() const { return nr; }
4744
inline int ncol() const { return nc; }
4845

4946
private:
50-
int nr, nc ;
51-
typedef bool (UpperTri::*Method)(int,int) ;
47+
typedef bool (UpperTri::*Method)(int, int) const;
5248

53-
Method getter ;
54-
inline bool get_diag_true( int i, int j ){
55-
return i >= j ;
56-
}
57-
inline bool get_diag_false( int i, int j ){
58-
return i > j ;
59-
}
60-
inline bool get( int i, int j){
61-
return (this->*getter)(i, j ) ;
62-
}
49+
int nr, nc;
50+
Method getter;
6351

64-
} ;
52+
inline bool get_diag_true(int i, int j) const { return i <= j; }
53+
54+
inline bool get_diag_false(int i, int j) const { return i < j; }
55+
56+
inline bool get(int i, int j) const { return (this->*getter)(i, j); }
57+
};
6558

6659
} // sugar
6760

68-
template <int RTYPE, bool LHS_NA, typename LHS_T>
69-
inline sugar::UpperTri<RTYPE,LHS_NA,LHS_T>
70-
upper_tri( const Rcpp::MatrixBase<RTYPE,LHS_NA,LHS_T>& lhs, bool diag = false){
71-
return sugar::UpperTri<RTYPE,LHS_NA,LHS_T>( lhs, diag ) ;
61+
template <int RTYPE, bool NA, typename T>
62+
inline sugar::UpperTri<RTYPE, NA, T>
63+
upper_tri(const Rcpp::MatrixBase<RTYPE, NA, T>& lhs, bool diag = false) {
64+
return sugar::UpperTri<RTYPE, NA, T>(lhs, diag);
7265
}
7366

7467
} // Rcpp
7568

76-
#endif
69+
#endif // Rcpp__sugar__upper_tri_h

0 commit comments

Comments
 (0)