Skip to content

Commit 631b73e

Browse files
committed
Investigate a couple bogus warnings
1 parent a72390f commit 631b73e

File tree

7 files changed

+48
-43
lines changed

7 files changed

+48
-43
lines changed

TODO

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ checkbox [C-ct] flip TODO
55

66
#+STARTUP: logdone
77

8-
* TODO [1/7]
9-
- [-] Bugs [5/16]
8+
* TODO [0/6]
9+
- [-] Bugs [1/12]
1010
- [ ] We shouldn't need ra::iota(3, ra::dim_t(4)) to get iota to use Seq<dim_t> instead of Seq<int>.
1111
- [ ] Fix reduction-1.cc: "real_part(iter<1>(m)) = real_part(iter<1>(c))"
1212
- [ ] Invalid expressions that aren't caught by Match, either bc it makes assumptions, or bc
@@ -27,7 +27,9 @@ checkbox [C-ct] flip TODO
2727
way.
2828
- [ ] be namespace clean except for possibly the scalar block in ra.hh. Document exactly what is exported to ::.
2929
- [ ] bad uses of assert [ra17]
30-
- [-] Features [3/17]
30+
- [ ] poor shape mismatch report, e.g. [1 2] vs [2] produces 'len[0] is -192...' (magic number for MISS)
31+
instead of e.g. 'len[0] MISS 1 vs 2'.
32+
- [-] Features [0/15]
3133
- [ ] generalize cat
3234
- [-] support std::format
3335
- [X] basic support
@@ -65,7 +67,7 @@ checkbox [C-ct] flip TODO
6567
- [X] Scalar / unbeatable <2023-08-04 Fri 14:18>
6668
- [X] iota. But iota args need to integral constants for this to work.
6769
- [ ] Optimize iota(len...) + iota(len...)
68-
- [-] Performance [2/4]
70+
- [-] Performance [0/2]
6971
- [ ] bench/bench-stencil* is weird.
7072
- [ ] Bigd cases in bench-at.
7173
- [-] Building, tests [4/6]
@@ -101,44 +103,48 @@ Some of these aren't bugs in the sense that I expect to solve them, but more lik
101103

102104
* Numbered issues through the source [ra..]
103105
1. [ ] test/bug10.cc
104-
2. [ ]
105-
3. [ ]
106+
2. [ ] test/maxwell.cc, fromb(terminal case) -Waggressive-loop-optimizations with gcc14/15 -DRA_CHECK=0 -O3.
107+
assert(ak>0) clears the warning, but [​[assume(ak>0)]] doesn't.
108+
3. [ ] test/fromu.cc, test/genfrom.cc,fromb(terminal case) warnings -Warray-bounds -Wstringop-overflow with
109+
gcc14/15 -DRA_CHECK=0 -O2 or -O3. assert(bk<ra::size(bv))) clears both warnings. [​[assume(...)]] clears
110+
-Wstringop-overflow but not -Warray-bounds.
106111
4. [ ] test/optimize.cc: Expression objects only keep pointers to data objects. This means that it
107-
is unsafe to define expression objects unless none of the data objects are
108-
temporaries. E.g. `auto e = temp + temp;` is unsafe. Either `obj a; obj b; auto e = a + b;` or
109-
`obj o = temp + temp;` is safe. Generally there's no reason to ever define expression objects
110-
explicitly.
112+
is unsafe to define expression objects unless none of the data objects are
113+
temporaries. E.g. `auto e = temp + temp;` is unsafe. Either `obj a; obj b; auto e = a + b;` or
114+
`obj o = temp + temp;` is safe. Generally there's no reason to ever define expression objects
115+
explicitly.
116+
1. [ ]
111117
5. [ ]
112118
6. [ ]
113-
7. [ ]
114-
8. [X] test/operators.cc: Some simple expressions with scalars fail in VAL(). The patch in ra.hh
119+
7. [X] test/operators.cc: Some simple expressions with scalars fail in VAL(). The patch in ra.hh
115120
triggers the address sanitizer in test/ra-9.cc.
116-
9. [ ] test/ra-6.cc: ra::Ptr doesn't hold copies. This enables restarting (see [ra39]), so
121+
8. [ ] test/ra-6.cc: ra::Ptr doesn't hold copies. This enables restarting (see [ra39]), so
117122
ra::ptr(temp) must only be used as temp. Really forbidding auto ll = ra::ptr(lvalue) would also
118123
be ok.
119-
10. [ ] test/ra-0.cc: size(SmallArray) requires ra:: to avoid collision with std::size, but not
120-
size(Big) (??).
124+
9. [ ] test/ra-0.cc: size(SmallArray) requires ra:: to avoid collision with std::size, but not
125+
size(Big) (??).
126+
10. [ ]
121127
11. [ ]
122-
12. [ ]
123-
13. [ ] ra/ply.hh (i/o), ra/expr.hh, ra/base.hh, test/compatibility.cc: std::string used to
128+
12. [ ] ra/ply.hh (i/o), ra/expr.hh, ra/base.hh, test/compatibility.cc: std::string used to
124129
be registered as scalar, but that clashes with how std::ranges sees it. OTOH we don't want
125130
format(std::string_view) to print it as a foreign vector, so we have an exception for it
126131
there. As things stand, you can register it as scalar or not.
127-
14. [ ] CellBig needs to copy its Dimv in some cases, which also complicates ViewBig::iter<>. Problem
132+
13. [ ] CellBig needs to copy its Dimv in some cases, which also complicates ViewBig::iter<>. Problem
128133
is demonstrated in ra-5.cc.
129-
15. [X] Conversion-to-scalar operators for dynamic-rank ViewBig(). I thought this could be
134+
14. [X] Conversion-to-scalar operators for dynamic-rank ViewBig(). I thought this could be
130135
https://wg21.link/cwg976 but gcc 14 doesn't fix it. There are two issues here, 1) why isn't
131136
const conversion enough and 2) ambiguity with Small's 'any' constructor (cf 'c++ converting
132137
constructor vs conversion operator').
133138
- <2025-06-17 Tue> This was fixed mostly by replacing the (auto && x) constructors in the
134139
array classes by (const & x). This may cause iterators to be copied, so maybe we can redo
135140
(auto && x) in the future if we find out it matters.
136-
16. [ ] Ambiguity in ravel vs nested constructors bc bc 1 converts to int2. Should be explicit
141+
15. [ ] Ambiguity in ravel vs nested constructors bc bc 1 converts to int2. Should be explicit
137142
(and ravel constructors also).
138-
17. [ ] assert() is used in some cases for runtime conditions that aren't ra::'s fault. This is
143+
16. [ ] assert() is used in some cases for runtime conditions that aren't ra::'s fault. This is
139144
bad because if RA_ASSERT is defined to throw, the caller probably expects to be able to handle
140145
the error. On the other hand if RA_CHECK is 0, we shouldn't continue. But ofc we do in
141146
index checks etc. So is this different?
147+
17. [ ]
142148
18. [ ]
143149
19. [ ]
144150
20. [ ]
@@ -153,28 +159,27 @@ Some of these aren't bugs in the sense that I expect to solve them, but more lik
153159
29. [ ]
154160
30. [ ]
155161
31. [ ]
156-
32. [ ]
157-
33. [X] test/frame-old.cc, test/fromb.cc: When mixing beaten & unbeaten subscripts and the
162+
32. [X] test/frame-old.cc, test/fromb.cc: When mixing beaten & unbeaten subscripts and the
158163
unbeaten subscripts go first, the result is a nested expression. This has to be fixed in the
159164
view operator()s. Fixed after rewrite of from() e9ed62aa3954dd2e3dc528c51da7269971f9c553
160165
<2025-08-22 Fri>.
166+
33. [ ]
161167
34. [ ]
162168
35. [ ]
163169
36. [ ]
164170
37. [ ]
165-
38. [ ]
166-
39. [ ] test/ra-9.cc: There's no reason to restart, since the Iterator methods are all static. So
171+
38. [ ] test/ra-9.cc: There's no reason to restart, since the Iterator methods are all static. So
167172
start(ra::Scalar) just fwds. But that means that Scalar::c maintain constness, so a const
168173
overload is required for ScalarFlat::operator*.
174+
39. [ ]
169175
40. [ ]
170-
41. [ ]
171-
42. [ ] Ways to test that compile time assertions or constraints are triggered appropriately,
176+
41. [ ] Ways to test that compile time assertions or constraints are triggered appropriately,
172177
esp. for things like ct array lookup. Some are doable with concepts, see e.g. ra-14.cc or
173178
big-0.cc.
179+
42. [ ]
174180
43. [ ]
175181
44. [ ]
176182
45. [ ]
177-
46. [ ]
178183

179184
* Old bugs or issues
180185
- [X] Before merging saveload [1/1]

examples/maxwell.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,9 @@ int main()
6767
constexpr int k = k_;
6868
const int o = DA.len(k);
6969
if (o>=2) {
70-
DA(HH<k>, iota(o-2, 1), HH<4-k>, k) = (A(HH<k>, iota(o-2, 2)) - A(HH<k>, iota(o-2, 0)));
71-
DA(HH<k>, 0, HH<4-k>, k) = (A(HH<k>, 1) - A(HH<k>, o-1));
72-
DA(HH<k>, o-1, HH<4-k>, k) = (A(HH<k>, 0) - A(HH<k>, o-2));
70+
DA(HH<k>, iota(o-2, 1), HH<4-k>, k) = A(HH<k>, iota(o-2, 2)) - A(HH<k>, iota(o-2, 0));
71+
DA(HH<k>, 0, HH<4-k>, k) = A(HH<k>, 1) - A(HH<k>, o-1);
72+
DA(HH<k>, o-1, HH<4-k>, k) = A(HH<k>, 0) - A(HH<k>, o-2);
7373
DA(HH<5>, k) *= factor;
7474
}
7575
};

ra/ply.hh

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -535,13 +535,13 @@ constexpr auto
535535
fromb(auto pl, auto ds, auto && bv, int bk, auto && a, int ak)
536536
{
537537
if constexpr (dobv) {
538-
#pragma GCC diagnostic push
539-
#pragma GCC diagnostic warning "-Waggressive-loop-optimizations" // gcc14/15.2 -DRA_CHECK=0 -O3
538+
#pragma GCC diagnostic push // gcc14/15 -DRA_CHECK=0 --no-sanitize -O3 [ra02]
539+
#pragma GCC diagnostic warning "-Waggressive-loop-optimizations"
540540
for (; ak<ra::rank(a); ++ak, ++bk) {
541541
#pragma GCC diagnostic pop
542-
#pragma GCC diagnostic push
543-
#pragma GCC diagnostic warning "-Warray-bounds" // gcc14/15.2 -DRA_CHECK=0 --no-sanitize -O2 -O3
544-
#pragma GCC diagnostic warning "-Wstringop-overflow" // gcc14/15.2 -DRA_CHECK=0 --no-sanitize -O2 -O3
542+
#pragma GCC diagnostic push // gcc14/15 -DRA_CHECK=0 --no-sanitize -O2 -O3 [ra03]
543+
#pragma GCC diagnostic warning "-Warray-bounds"
544+
#pragma GCC diagnostic warning "-Wstringop-overflow"
545545
bv[bk] = a.dimv[ak];
546546
#pragma GCC diagnostic pop
547547
}

test/bug10.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
using std::cout;
1515

1616
// FIXME [ra01] bug only shows up with all of: --no-sanitize -O>2, no assert (1)
17-
// see the patch in ViewSmall(nested braces)
17+
// patched with a barrier in ViewSmall(nested braces)
1818

1919
int main()
2020
{

test/fromu.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,13 @@ int main()
9898
tr.info("a(rank0, rank1)").test_eq(ra::Small<real, 2>{9, 7}, from(a, 1, Vint{1, 0}));
9999
tr.info("a(rank1, rank0)").test_eq(ra::Small<real, 2>{9, 1}, from(a, Vint{1, 0}, ra::Small<int>(1)));
100100
tr.info("a(rank0, rank1)").test_eq(ra::Small<real, 2>{9, 7}, from(a, ra::Small<int>(1), Vint{1, 0}));
101-
// using .iter() wass necessary when mixed u/b weren't accepted. Now rank 0 are always beaten, so it doesn't matter.
101+
// needed .iter() when mixed u/b weren't accepted. Now rank 0 are always beaten, so it doesn't matter.
102102
tr.info("a(rank1, rank0)").test_eq(ra::Small<real, 2>{9, 1}, from(a, Vint{1, 0}, ra::Small<int>(1).iter()));
103103
tr.info("a(rank0, rank1)").test_eq(ra::Small<real, 2>{9, 7}, from(a, ra::Small<int>(1).iter(), Vint{1, 0}));
104104
};
105105
check_selection_unbeatable_2(Ureal<2>({2, 2}, {1, 2, 3, 4}));
106106
check_selection_unbeatable_2(ra::Small<real, 2, 2>({1, 2, 3, 4}));
107-
check_selection_unbeatable_2(Ureal<>({2, 2}, {1, 2, 3, 4}));
107+
check_selection_unbeatable_2(Ureal<>({2, 2}, {1, 2, 3, 4})); // [ra03]
108108
}
109109
tr.section("mixed scalar/unbeatable, 2D -> 1D");
110110
{

test/genfrom.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ int main()
350350
tr.section("unbeaten ViewBig<... ANY> IIb");
351351
{
352352
auto i = ra::Small<int, 2, 3> {{3, 2, 1}, {4, 5, 6}};
353-
auto b = from(ra::ViewBig<ra::Seq<ra::dim_t>, ra::ANY>(ra::ii({10, 2})), i, 0);
353+
auto b = from(ra::ViewBig<ra::Seq<ra::dim_t>, ra::ANY>(ra::ii({10, 2})), i, 0); // [ra03]
354354
ra::Small<int, 2, 3> c = b;
355355
println(cout, "c {:c}", c);
356356
tr.strict().test_eq(ra::Small<int, 2, 3> {{6, 4, 2}, {8, 10, 12}}, c);

test/io.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ int main()
199199
o << fmt(ra::nstyle, A);
200200
tr.test_eq(std::string("1 2\n3 4"), o.str());
201201
}
202-
tr.section("[ra02a] printing Map");
202+
tr.section("printing Map");
203203
{
204204
iocheck(tr.info("output of map (1)"),
205205
ra::map_([](double i) { return -i; }, start(ra::Small<double, 3>{0, 1, 2})),
@@ -214,7 +214,7 @@ int main()
214214
ra::map_([](int i) { return -i; }, a.iter()),
215215
ra::Unique<int, 2>({2, 3}, { -1, -2, -3, -4, -5, -6 }));
216216
}
217-
tr.section("[ra02b] printing array iterators");
217+
tr.section("printing array iterators");
218218
{
219219
ra::Unique<int, 2> a({3, 2}, { 1, 2, 3, 4, 5, 6 });
220220
iocheck(tr.info("output of array through its iterator"), a.iter(), a);
@@ -223,7 +223,7 @@ int main()
223223
transpose(a, {1, 0}).iter(),
224224
ra::Unique<int>({2, 3}, { 1, 3, 5, 2, 4, 6 }));
225225
}
226-
tr.section("[ra02c] printing array iterators");
226+
tr.section("printing array iterators");
227227
{
228228
ra::Small<int, 3, 2> a { 1, 2, 3, 4, 5, 6 };
229229
iocheck(tr.info("output of array through its iterator"), a.iter(), a);

0 commit comments

Comments
 (0)