Skip to content

Commit 709d194

Browse files
carsonRadtkecarsonRadtke
committed
address a number of PR comments
1 parent e52da3f commit 709d194

File tree

4 files changed

+36
-32
lines changed

4 files changed

+36
-32
lines changed

‎include/gsl/dyn_array‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ namespace details
4242
using reference = T&;
4343
using const_reference = const T&;
4444
using difference_type = std::ptrdiff_t;
45-
using size_type = decltype(sizeof(0));
45+
using size_type = std::size_t;
4646
};
4747

4848
template <typename T, typename Allocator = std::allocator<T>>
@@ -51,6 +51,7 @@ namespace details
5151
using pointer = typename dyn_array_traits<T>::pointer;
5252
using size_type = typename dyn_array_traits<T>::size_type;
5353

54+
protected:
5455
const class dyn_array_impl
5556
{
5657
public:
@@ -76,10 +77,8 @@ namespace details
7677

7778
GSL_CONSTEXPR_SINCE_CPP20 ~dyn_array_base()
7879
{
79-
if (impl().data()) Allocator::deallocate(impl().data(), impl().count());
80+
if (_impl.data()) Allocator::deallocate(_impl.data(), _impl.count());
8081
}
81-
82-
constexpr auto impl() const -> const dyn_array_impl& { return _impl; }
8382
};
8483

8584
template <typename T>
@@ -96,7 +95,7 @@ namespace details
9695
using iterator_category = std::random_access_iterator_tag;
9796

9897
#ifdef GSL_HAS_RANGES
99-
constexpr dyn_array_iterator() : dyn_array_iterator(nullptr, 0, 0) {}
98+
constexpr dyn_array_iterator() = default;
10099
#endif /* GSL_HAS_RANGES */
101100

102101
constexpr dyn_array_iterator(pointer ptr, size_type pos, size_type end_pos)
@@ -134,7 +133,7 @@ namespace details
134133
constexpr auto operator++() -> dyn_array_iterator&
135134
{
136135
Expects(_pos < _end_pos);
137-
_pos++;
136+
++_pos;
138137
return *this;
139138
}
140139

@@ -148,7 +147,7 @@ namespace details
148147
constexpr auto operator--() -> dyn_array_iterator&
149148
{
150149
Expects(_pos > 0);
151-
_pos--;
150+
--_pos;
152151
return *this;
153152
}
154153

@@ -161,17 +160,19 @@ namespace details
161160

162161
constexpr auto operator+=(difference_type diff) -> dyn_array_iterator&
163162
{
164-
auto pos = gsl::narrow<difference_type>(_pos);
165-
Expects(pos + diff <= gsl::narrow<difference_type>(_end_pos));
166-
_pos = gsl::narrow<size_type>(pos + diff);
163+
auto new_pos = gsl::narrow<difference_type>(_pos) + diff;
164+
Expects(new_pos >= 0);
165+
Expects(new_pos <= gsl::narrow<difference_type>(_end_pos));
166+
_pos = gsl::narrow<size_type>(new_pos);
167167
return *this;
168168
}
169169

170170
constexpr auto operator-=(difference_type diff) -> dyn_array_iterator&
171171
{
172-
auto pos = gsl::narrow<difference_type>(_pos);
173-
Expects(pos >= diff);
174-
_pos = gsl::narrow<size_type>(pos - diff);
172+
auto new_pos = gsl::narrow<difference_type>(_pos) - diff;
173+
Expects(new_pos >= 0);
174+
Expects(new_pos <= gsl::narrow<difference_type>(_end_pos));
175+
_pos = gsl::narrow<size_type>(new_pos);
175176
return *this;
176177
}
177178

@@ -204,14 +205,14 @@ namespace details
204205
}
205206

206207
private:
207-
pointer _ptr;
208-
size_type _pos;
209-
size_type _end_pos;
208+
pointer _ptr{};
209+
size_type _pos{};
210+
size_type _end_pos{};
210211
};
211212
} // namespace details
212213

213214
template <typename T, typename Allocator = std::allocator<T>>
214-
class dyn_array : public details::dyn_array_base<T, Allocator>
215+
class dyn_array : private details::dyn_array_base<T, Allocator>
215216
{
216217
using base = details::dyn_array_base<T, Allocator>;
217218
using pointer = typename details::dyn_array_traits<T>::pointer;
@@ -231,7 +232,7 @@ public:
231232

232233
explicit constexpr dyn_array(const Allocator& alloc = {}) : base{alloc} {}
233234

234-
explicit constexpr dyn_array(size_type count, const T& value, const Allocator& alloc = {})
235+
constexpr dyn_array(size_type count, const T& value, const Allocator& alloc = {})
235236
: base{count, alloc}
236237
{
237238
std::fill(begin(), end(), value);
@@ -278,7 +279,7 @@ public:
278279

279280
constexpr auto operator!=(const dyn_array& other) const { return !(*this == other); }
280281

281-
constexpr auto size() const { return impl().count(); }
282+
constexpr auto size() const { return base::_impl.count(); }
282283

283284
constexpr auto empty() const { return size() == 0; }
284285

@@ -297,14 +298,16 @@ public:
297298
return const_cast<dyn_array&>(*this)[pos];
298299
}
299300

300-
constexpr auto data() { return impl().data(); }
301+
constexpr auto data() { return base::_impl.data(); }
301302
constexpr auto data() const -> const T* { return const_cast<dyn_array&>(*this).data(); }
302303

303304
constexpr auto begin() { return iterator{data(), 0, size()}; }
304305
constexpr auto begin() const { return const_iterator{data(), 0, size()}; }
306+
constexpr auto cbegin() const { return begin(); }
305307

306308
constexpr auto rbegin() { return reverse_iterator{end()}; }
307309
constexpr auto rbegin() const { return const_reverse_iterator{end()}; }
310+
constexpr auto crbegin() const { return rbegin(); }
308311

309312
#ifdef _MSC_VER
310313
constexpr auto _Unchecked_begin() { return data(); }
@@ -316,9 +319,11 @@ public:
316319

317320
constexpr auto end() { return iterator{data(), size(), size()}; }
318321
constexpr auto end() const { return const_iterator{data(), size(), size()}; }
322+
constexpr auto cend() const { return end(); }
319323

320324
constexpr auto rend() { return reverse_iterator{begin()}; }
321325
constexpr auto rend() const { return const_reverse_iterator{begin()}; }
326+
constexpr auto crend() const { return rend(); }
322327

323328
#ifdef _MSC_VER
324329
constexpr auto _Unchecked_end() { return data() + size(); }
@@ -327,9 +332,6 @@ public:
327332
return const_cast<dyn_array&>(*this)._Unchecked_end();
328333
}
329334
#endif /* MSC_VER */
330-
331-
private:
332-
constexpr auto impl() const { return base::impl(); }
333335
};
334336

335337
#ifdef GSL_HAS_DEDUCTION_GUIDES

‎include/gsl/util‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@
113113
template <typename Ty> \
114114
requires(std::input_iterator<Ty>)
115115
#else // ^^^ since C++20 /// before C++20 vvv
116-
#define GSL_UNTIL_CPP20
116+
#define GSL_BEFORE_CPP20
117117
#define GSL_CONSTEXPR_SINCE_CPP20
118118
#define GSL_TYPE_IS_ITERATOR(Ty) \
119119
template <typename Ty, std::enable_if_t<details::is_iterator<Ty>::value, bool> = true>
@@ -128,7 +128,7 @@ namespace gsl
128128
// index type for all container indexes/subscripts/sizes
129129
using index = std::ptrdiff_t;
130130

131-
#ifdef GSL_UNTIL_CPP20
131+
#ifdef GSL_BEFORE_CPP20
132132
namespace details
133133
{
134134
template <typename ...>
@@ -142,7 +142,7 @@ namespace details
142142
struct is_iterator<T, void_t<typename std::iterator_traits<T>::value_type>>
143143
: std::true_type {};
144144
} // namespace details
145-
#endif /* GSL_UNTIL_CPP20 */
145+
#endif /* GSL_BEFORE_CPP20 */
146146

147147
// final_action allows you to ensure something gets run at the end of a scope
148148
template <class F>

‎tests/CMakeLists.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ include(CheckCXXCompilerFlag)
7676
# please try to keep entries ordered =)
7777
add_library(gsl_tests_config INTERFACE)
7878
if(MSVC) # MSVC or simulating MSVC
79+
set(CMAKE_VS_GLOBALS
80+
"EnableMicrosoftCodeAnalysis=true"
81+
"CodeAnalysisRuleSet=AllRules.ruleset"
82+
)
7983
target_compile_options(gsl_tests_config INTERFACE
8084
${GSL_CPLUSPLUS_OPT}
8185
/EHsc

‎tests/dyn_array_tests.cpp‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ TEST(dyn_array_tests, ranges)
167167

168168
#ifdef GSL_HAS_CONTAINER_RANGES
169169
std::vector<char> twins(10, 'c');
170-
gsl::dyn_array<char> mets(std::from_range_t{}, twins);
170+
gsl::dyn_array<char> mets(std::from_range, twins);
171171
EXPECT_EQ(twins.size(), mets.size());
172172
EXPECT_TRUE(std::ranges::all_of(mets, [](char c) { return c == 'c'; }));
173173
#endif /* GSL_HAS_CONTAINER_RANGES */
@@ -178,10 +178,8 @@ TEST(dyn_array_tests, ranges)
178178
template <typename T, unsigned N>
179179
struct ConstexprAllocator
180180
{
181-
T buf[N];
182-
std::size_t sz;
183-
184-
constexpr ConstexprAllocator() : buf{}, sz{} {}
181+
T buf[N]{};
182+
std::size_t sz{};
185183

186184
constexpr auto allocate(std::size_t n)
187185
{
@@ -283,7 +281,7 @@ TEST(dyn_array_tests, deduction_guides)
283281
{
284282
std::vector<char> giants{10};
285283
#ifdef GSL_HAS_CONTAINER_RANGES
286-
gsl::dyn_array mariners(std::from_range_t{}, giants);
284+
gsl::dyn_array mariners(std::from_range, giants);
287285
#endif /* GSL_HAS_CONTAINER_RANGES */
288286
gsl::dyn_array cardinals(std::begin(giants), std::end(giants));
289287
}

0 commit comments

Comments
 (0)