Avoid forwarding a bounds-checked subscript operation, closes #799

hsutter · hsutter · commit 0b333f3b6da1 · 2023-11-20T14:42:19.000-08:00
diff --git a/include/cpp2util.h b/include/cpp2util.h
@@ -457,22 +457,22 @@ auto assert_not_null(auto&& p CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> declty
 
 //  Subscript bounds checking
 //
-auto assert_in_bounds(auto&& x, auto&& arg CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> decltype(auto)
+auto assert_in_bounds_impl(auto&& x, auto&& arg CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> void
     requires (std::is_integral_v<CPP2_TYPEOF(arg)> &&
              requires { std::size(x); std::ssize(x); x[arg]; std::begin(x) + 2; })
 {
     Bounds.expects(0 <= arg && arg < [&]() -> auto {
         if constexpr (std::is_signed_v<CPP2_TYPEOF(arg)>) { return std::ssize(x); }
         else { return std::size(x); }
     }(), "out of bounds access attempt detected" CPP2_SOURCE_LOCATION_ARG);
-    return CPP2_FORWARD(x) [ CPP2_FORWARD(arg) ];
 }
 
-auto assert_in_bounds(auto&& x, auto&& arg CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> decltype(auto)
+auto assert_in_bounds_impl(auto&&, auto&& CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> void
 {
-    return CPP2_FORWARD(x) [ CPP2_FORWARD(arg) ];
 }
 
+#define CPP2_ASSERT_IN_BOUNDS(x, arg) (cpp2::assert_in_bounds_impl((x),(arg)), (x)[(arg)])
+
 
 //-----------------------------------------------------------------------
 //
diff --git a/regression-tests/test-results/mixed-bounds-check.cpp b/regression-tests/test-results/mixed-bounds-check.cpp
@@ -27,6 +27,6 @@
 
     std::vector v {1, 2, 3, 4, 5, -999}; 
     CPP2_UFCS_0(pop_back, v);
-    std::cout << cpp2::assert_in_bounds(std::move(v), 5) << "\n";
+    std::cout << CPP2_ASSERT_IN_BOUNDS(std::move(v), 5) << "\n";
 }
 
diff --git a/regression-tests/test-results/mixed-test-parens.cpp b/regression-tests/test-results/mixed-test-parens.cpp
@@ -28,7 +28,7 @@ constexpr int a = 1;
 #line 8 "mixed-test-parens.cpp2"
 [[nodiscard]] auto main() -> int{
     std::vector<int> v {1, 2, 3}; 
-    std::cout << (1 + 2) * (3 + cpp2::assert_in_bounds(std::move(v), 0));
+    std::cout << (1 + 2) * (3 + CPP2_ASSERT_IN_BOUNDS(std::move(v), 0));
     f<(cpp2::cmp_greater(1,2))>(3, 4);
     f<a + a>(5, 6);
 }
diff --git a/regression-tests/test-results/pure2-bounds-safety-span.cpp b/regression-tests/test-results/pure2-bounds-safety-span.cpp
@@ -36,7 +36,7 @@ auto print_and_decorate(auto const& thing) -> void;
 
     auto i {0}; 
     for( ; cpp2::cmp_less(i,CPP2_UFCS_0(ssize, s)); ++i ) {
-        print_and_decorate(cpp2::assert_in_bounds(s, i));
+        print_and_decorate(CPP2_ASSERT_IN_BOUNDS(s, i));
     }
 }
 
diff --git a/regression-tests/test-results/pure2-bugfix-for-non-local-initialization.cpp b/regression-tests/test-results/pure2-bugfix-for-non-local-initialization.cpp
@@ -28,7 +28,7 @@ auto main() -> int;
 
 #line 5 "pure2-bugfix-for-non-local-initialization.cpp2"
 auto main() -> int{
-  cpp2::Testing.expects(cpp2::assert_in_bounds(t::value, 0) == 17, "");
-  cpp2::Testing.expects(cpp2::assert_in_bounds(t::value, 1) == 29, "");
+  cpp2::Testing.expects(CPP2_ASSERT_IN_BOUNDS(t::value, 0) == 17, "");
+  cpp2::Testing.expects(CPP2_ASSERT_IN_BOUNDS(t::value, 1) == 29, "");
 }
 
diff --git a/regression-tests/test-results/pure2-intro-example-three-loops.cpp b/regression-tests/test-results/pure2-intro-example-three-loops.cpp
@@ -45,7 +45,7 @@ auto decorate_and_print(auto& thing) -> void{
 
     auto i {cpp2_new<int>(0)}; 
     for( ; cpp2::cmp_less(*cpp2::assert_not_null(i),CPP2_UFCS_0(ssize, view)); ++*cpp2::assert_not_null(i) ) {
-        print(cpp2::assert_in_bounds(view, *cpp2::assert_not_null(i)));
+        print(CPP2_ASSERT_IN_BOUNDS(view, *cpp2::assert_not_null(i)));
     }
 
     do {
diff --git a/regression-tests/test-results/pure2-main-args.cpp b/regression-tests/test-results/pure2-main-args.cpp
@@ -22,5 +22,5 @@ auto main(int const argc_, char** argv_) -> int {
 #line 2 "pure2-main-args.cpp2"
     std::cout 
         << "args.argc            is " + cpp2::to_string(args.argc) + "\n" 
-        << "args.argv[0]         is " + cpp2::to_string(cpp2::assert_in_bounds(args.argv, 0)) + "\n"; }
+        << "args.argv[0]         is " + cpp2::to_string(CPP2_ASSERT_IN_BOUNDS(args.argv, 0)) + "\n"; }
 
diff --git a/regression-tests/test-results/pure2-print.cpp b/regression-tests/test-results/pure2-print.cpp
@@ -102,7 +102,7 @@ requires (true) inline CPP2_CONSTEXPR T outer::object_alias = 42;
             ret += strlen(s) - 10 + CPP2_UFCS_0(strlen, std::move(s)) * (16 / (3 & 2)) % 3;
 
             map<int const,string> m {}; 
-            cpp2::assert_in_bounds(m, 0) = cpp2::as_<string>("har");
+            CPP2_ASSERT_IN_BOUNDS(m, 0) = cpp2::as_<string>("har");
             ret -= CPP2_UFCS_0(length, h("x", m));
             static_cast<void>(std::move(m));
 
@@ -133,7 +133,7 @@ requires (true) inline CPP2_CONSTEXPR T outer::object_alias = 42;
 
             cpp2::Default.expects(true, "");
 
-            return [_0 = (s + cpp2::assert_in_bounds(m, 0))]() -> std::string { return _0;  }(); 
+            return [_0 = (s + CPP2_ASSERT_IN_BOUNDS(m, 0))]() -> std::string { return _0;  }(); 
         }
 
         template<typename T> [[nodiscard]] auto outer::mytype::values([[maybe_unused]] T const& unnamed_param_2) const& -> values_ret{
diff --git a/regression-tests/test-results/pure2-raw-string-literal-and-interpolation.cpp b/regression-tests/test-results/pure2-raw-string-literal-and-interpolation.cpp
@@ -21,8 +21,8 @@
 #line 2 "pure2-raw-string-literal-and-interpolation.cpp2"
     auto i {42}; 
     std::map<std::string,int> m {}; 
-    cpp2::assert_in_bounds(m, "one") = 1;
-    cpp2::assert_in_bounds(m, "two") = 2;
+    CPP2_ASSERT_IN_BOUNDS(m, "one") = 1;
+    CPP2_ASSERT_IN_BOUNDS(m, "two") = 2;
 
     std::string str {"this is a string"}; 
 
@@ -38,7 +38,7 @@ lines)test"};
 that can last for multiple
 lines
 )test" + cpp2::to_string(i) + R"test( R"(this can be added too)"
-calculations like m["one"] + m["two"] = )test" + cpp2::to_string(cpp2::assert_in_bounds(m, "one") + cpp2::assert_in_bounds(m, "two")) + R"test( also works
+calculations like m["one"] + m["two"] = )test" + cpp2::to_string(CPP2_ASSERT_IN_BOUNDS(m, "one") + CPP2_ASSERT_IN_BOUNDS(m, "two")) + R"test( also works
 )test" + cpp2::to_string("at the beginning of the line") +  R"test(!!!)test"}; 
 
     std::string raw_str_inter_multi {R"(
@@ -50,6 +50,6 @@ calculations like m["one"] + m["two"] = )test" + cpp2::to_string(cpp2::assert_in
     std::cout << std::move(raw_str_multi) << std::endl;
     std::cout << std::move(raw_str_inter) << std::endl;
     std::cout << std::move(raw_str_inter_multi) << std::endl;
-    std::cout << (cpp2::to_string(cpp2::assert_in_bounds(m, "one")) + R"(.)" + cpp2::to_string(cpp2::assert_in_bounds(m, "two")) + R"(.)" + cpp2::to_string(cpp2::assert_in_bounds(std::move(m), "three")) + R"(.)" + cpp2::to_string(std::move(i))) << std::endl;
+    std::cout << (cpp2::to_string(CPP2_ASSERT_IN_BOUNDS(m, "one")) + R"(.)" + cpp2::to_string(CPP2_ASSERT_IN_BOUNDS(m, "two")) + R"(.)" + cpp2::to_string(CPP2_ASSERT_IN_BOUNDS(std::move(m), "three")) + R"(.)" + cpp2::to_string(std::move(i))) << std::endl;
 }
 
diff --git a/regression-tests/test-results/version b/regression-tests/test-results/version
@@ -1,5 +1,5 @@
 
-cppfront compiler v0.3.0   Build 8B19:1018
+cppfront compiler v0.3.0   Build 8B20:1424
 Copyright(c) Herb Sutter   All rights reserved
 
 SPDX-License-Identifier: CC-BY-NC-ND-4.0
diff --git a/source/build.info b/source/build.info
@@ -1 +1 @@
-"8B19:1018"
+"8B20:1424"
diff --git a/source/reflect.h b/source/reflect.h
@@ -753,7 +753,7 @@ namespace meta {
     [[nodiscard]] auto compiler_services::get_argument(cpp2::in<int> index) & -> std::string{
         metafunctions_used = true;
         if (([_0 = 0, _1 = index, _2 = CPP2_UFCS_0(ssize, metafunction_args)]{ return cpp2::cmp_less_eq(_0,_1) && cpp2::cmp_less(_1,_2); }())) {
-            return cpp2::assert_in_bounds(metafunction_args, index); 
+            return CPP2_ASSERT_IN_BOUNDS(metafunction_args, index); 
         }
         return ""; 
     }
@@ -1395,7 +1395,7 @@ std::string value = "-1";
 
         nextval(value, init);
 
-        auto v {std::strtoll(&cpp2::assert_in_bounds(value, 0), nullptr, 10)}; // for non-numeric values we'll just get 0 which is okay for now
+        auto v {std::strtoll(&CPP2_ASSERT_IN_BOUNDS(value, 0), nullptr, 10)}; // for non-numeric values we'll just get 0 which is okay for now
         if (cpp2::cmp_less(v,min_value)) {
             min_value = v;
         }
@@ -1535,7 +1535,7 @@ auto cpp2_enum(meta::type_declaration& t) -> void
             if (!(CPP2_UFCS_0(empty, specified_value))) {
                 value = specified_value;
             }else {
-                auto v {std::strtoll(&cpp2::assert_in_bounds(value, 0), nullptr, 10)}; 
+                auto v {std::strtoll(&CPP2_ASSERT_IN_BOUNDS(value, 0), nullptr, 10)}; 
                 value = cpp2::as_<std::string>((std::move(v) + 1));
             }
         }, 
@@ -1552,7 +1552,7 @@ auto flag_enum(meta::type_declaration& t) -> void
             if (!(CPP2_UFCS_0(empty, specified_value))) {
                 value = specified_value;
             }else {
-                auto v {std::strtoll(&cpp2::assert_in_bounds(value, 0), nullptr, 10)}; 
+                auto v {std::strtoll(&CPP2_ASSERT_IN_BOUNDS(value, 0), nullptr, 10)}; 
                 if (cpp2::cmp_less(v,1)) {
                     value = "1";
                 }
@@ -1810,7 +1810,7 @@ auto print(cpp2::in<meta::type_declaration> t) -> void
             && !(CPP2_UFCS_0(arguments_were_used, rtype)))) 
 
         {
-            error(name + " did not use its template arguments - did you mean to write '" + name + " <" + cpp2::assert_in_bounds(args, 0) + "> type' (with the spaces)?");
+            error(name + " did not use its template arguments - did you mean to write '" + name + " <" + CPP2_ASSERT_IN_BOUNDS(args, 0) + "> type' (with the spaces)?");
             return false; 
         }
     }
diff --git a/source/to_cpp1.h b/source/to_cpp1.h
@@ -3178,7 +3178,7 @@ class cppfront
                     && std::ssize(i->expr_list->expressions) == 1
                     )
                 {
-                    prefix.emplace_back( "cpp2::assert_in_bounds(", i->op->position() );
+                    prefix.emplace_back( "CPP2_ASSERT_IN_BOUNDS(", i->op->position() );
                     suffix.emplace_back( ", ", i->op->position() );
                 }
                 else {
@@ -5342,7 +5342,7 @@ class cppfront
 
                 auto func_name = n.name()->to_string();
 
-                //  If it's a single named value, emit it as an aonymous return value
+                //  If it's a single named value, emit it as an anonymous return value
                 if (std::ssize(r->parameters) == 1)
                 {
                     printer.print_extra(

Original file line number	Diff line number	Diff line change
`@@ -457,22 +457,22 @@ auto assert_not_null(auto&& p CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> declty`
`457`	`457`
`458`	`458`	`// Subscript bounds checking`
`459`	`459`	`//`
`460`		`-auto assert_in_bounds(auto&& x, auto&& arg CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> decltype(auto)`
	`460`	`+auto assert_in_bounds_impl(auto&& x, auto&& arg CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> void`
`461`	`461`	`requires (std::is_integral_v<CPP2_TYPEOF(arg)> &&`
`462`	`462`	`requires { std::size(x); std::ssize(x); x[arg]; std::begin(x) + 2; })`
`463`	`463`	`{`
`464`	`464`	`Bounds.expects(0 <= arg && arg < [&]() -> auto {`
`465`	`465`	`if constexpr (std::is_signed_v<CPP2_TYPEOF(arg)>) { return std::ssize(x); }`
`466`	`466`	`else { return std::size(x); }`
`467`	`467`	`}(), "out of bounds access attempt detected" CPP2_SOURCE_LOCATION_ARG);`
`468`		`- return CPP2_FORWARD(x) [ CPP2_FORWARD(arg) ];`
`469`	`468`	`}`
`470`	`469`
`471`		`-auto assert_in_bounds(auto&& x, auto&& arg CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> decltype(auto)`
	`470`	`+auto assert_in_bounds_impl(auto&&, auto&& CPP2_SOURCE_LOCATION_PARAM_WITH_DEFAULT) -> void`
`472`	`471`	`{`
`473`		`- return CPP2_FORWARD(x) [ CPP2_FORWARD(arg) ];`
`474`	`472`	`}`
`475`	`473`
	`474`	`+#define CPP2_ASSERT_IN_BOUNDS(x, arg) (cpp2::assert_in_bounds_impl((x),(arg)), (x)[(arg)])`
	`475`	`+`
`476`	`476`
`477`	`477`	`//-----------------------------------------------------------------------`
`478`	`478`	`//`
Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,6 @@`
`27`	`27`
`28`	`28`	`std::vector v {1, 2, 3, 4, 5, -999};`
`29`	`29`	`CPP2_UFCS_0(pop_back, v);`
`30`		`- std::cout << cpp2::assert_in_bounds(std::move(v), 5) << "\n";`
	`30`	`+ std::cout << CPP2_ASSERT_IN_BOUNDS(std::move(v), 5) << "\n";`
`31`	`31`	`}`
`32`	`32`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ constexpr int a = 1;`
`28`	`28`	`#line 8 "mixed-test-parens.cpp2"`
`29`	`29`	`[[nodiscard]] auto main() -> int{`
`30`	`30`	`std::vector<int> v {1, 2, 3};`
`31`		`- std::cout << (1 + 2) * (3 + cpp2::assert_in_bounds(std::move(v), 0));`
	`31`	`+ std::cout << (1 + 2) * (3 + CPP2_ASSERT_IN_BOUNDS(std::move(v), 0));`
`32`	`32`	`f<(cpp2::cmp_greater(1,2))>(3, 4);`
`33`	`33`	`f<a + a>(5, 6);`
`34`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ auto print_and_decorate(auto const& thing) -> void;`
`36`	`36`
`37`	`37`	`auto i {0};`
`38`	`38`	`for( ; cpp2::cmp_less(i,CPP2_UFCS_0(ssize, s)); ++i ) {`
`39`		`- print_and_decorate(cpp2::assert_in_bounds(s, i));`
	`39`	`+ print_and_decorate(CPP2_ASSERT_IN_BOUNDS(s, i));`
`40`	`40`	`}`
`41`	`41`	`}`
`42`	`42`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ auto main() -> int;`
`28`	`28`
`29`	`29`	`#line 5 "pure2-bugfix-for-non-local-initialization.cpp2"`
`30`	`30`	`auto main() -> int{`
`31`		`- cpp2::Testing.expects(cpp2::assert_in_bounds(t::value, 0) == 17, "");`
`32`		`- cpp2::Testing.expects(cpp2::assert_in_bounds(t::value, 1) == 29, "");`
	`31`	`+ cpp2::Testing.expects(CPP2_ASSERT_IN_BOUNDS(t::value, 0) == 17, "");`
	`32`	`+ cpp2::Testing.expects(CPP2_ASSERT_IN_BOUNDS(t::value, 1) == 29, "");`
`33`	`33`	`}`
`34`	`34`
Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ auto decorate_and_print(auto& thing) -> void{`
`45`	`45`
`46`	`46`	`auto i {cpp2_new<int>(0)};`
`47`	`47`	`for( ; cpp2::cmp_less(cpp2::assert_not_null(i),CPP2_UFCS_0(ssize, view)); ++cpp2::assert_not_null(i) ) {`
`48`		`- print(cpp2::assert_in_bounds(view, *cpp2::assert_not_null(i)));`
	`48`	`+ print(CPP2_ASSERT_IN_BOUNDS(view, *cpp2::assert_not_null(i)));`
`49`	`49`	`}`
`50`	`50`
`51`	`51`	`do {`