base: replace raw pointers for out-parameters with safer out<T>
Add a zero-cost type-safe abstraction for representing "out" parameters
(i.e. when the calling function has to return multiple results out
by-reference into the argument slots instead of using the return slot).
Change-Id: I33a941e4863b6bed71d2bfa43d7f48e9b111f83f
diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk
index 63ad9cf..4850e6c 100644
--- a/build/Android.gtest.mk
+++ b/build/Android.gtest.mk
@@ -165,6 +165,7 @@
runtime/base/hex_dump_test.cc \
runtime/base/histogram_test.cc \
runtime/base/mutex_test.cc \
+ runtime/base/out_test.cc \
runtime/base/scoped_flock_test.cc \
runtime/base/stringprintf_test.cc \
runtime/base/time_utils_test.cc \
diff --git a/runtime/base/out.h b/runtime/base/out.h
new file mode 100644
index 0000000..7199b63
--- /dev/null
+++ b/runtime/base/out.h
@@ -0,0 +1,274 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef ART_RUNTIME_BASE_OUT_H_
+#define ART_RUNTIME_BASE_OUT_H_
+
+#include <base/macros.h>
+#include <base/logging.h>
+
+#include <memory>
+// A zero-overhead abstraction marker that means this value is meant to be used as an out
+// parameter for functions. It mimics semantics of a pointer that the function will
+// dereference and output its value into.
+//
+// Inspired by the 'out' language keyword in C#.
+//
+// Declaration example:
+// int do_work(size_t args, out<int> result);
+// // returns 0 on success, sets result, otherwise error code
+//
+// Use-site example:
+// // (1) -- out of a local variable or field
+// int res;
+// if (do_work(1, outof(res)) {
+// cout << "success: " << res;
+// }
+// // (2) -- out of an iterator
+// std::vector<int> list = {1};
+// std::vector<int>::iterator it = list.begin();
+// if (do_work(2, outof_iterator(*it)) {
+// cout << "success: " << list[0];
+// }
+// // (3) -- out of a pointer
+// int* array = &some_other_value;
+// if (do_work(3, outof_ptr(array))) {
+// cout << "success: " << *array;
+// }
+//
+// The type will also automatically decay into a C-style pointer for compatibility
+// with calling legacy code that expect pointers.
+//
+// Declaration example:
+// void write_data(int* res) { *res = 5; }
+//
+// Use-site example:
+// int data;
+// write_data(outof(res));
+// // data is now '5'
+// (The other outof_* functions can be used analogously when the target is a C-style pointer).
+//
+// ---------------
+//
+// Other typical pointer operations such as addition, subtraction, etc are banned
+// since there is exactly one value being output.
+//
+namespace art {
+
+// Forward declarations. See below for specific functions.
+template <typename T>
+struct out_convertible; // Implicitly converts to out<T> or T*.
+
+// Helper function that automatically infers 'T'
+//
+// Returns a type that is implicitly convertible to either out<T> or T* depending
+// on the call site.
+//
+// Example:
+// int do_work(size_t args, out<int> result);
+// // returns 0 on success, sets result, otherwise error code
+//
+// Usage:
+// int res;
+// if (do_work(1, outof(res)) {
+// cout << "success: " << res;
+// }
+template <typename T>
+out_convertible<T> outof(T& param) ALWAYS_INLINE;
+
+// Helper function that automatically infers 'T' from a container<T>::iterator.
+// To use when the argument is already inside an iterator.
+//
+// Returns a type that is implicitly convertible to either out<T> or T* depending
+// on the call site.
+//
+// Example:
+// int do_work(size_t args, out<int> result);
+// // returns 0 on success, sets result, otherwise error code
+//
+// Usage:
+// std::vector<int> list = {1};
+// std::vector<int>::iterator it = list.begin();
+// if (do_work(2, outof_iterator(*it)) {
+// cout << "success: " << list[0];
+// }
+template <typename It>
+auto ALWAYS_INLINE outof_iterator(It iter)
+ -> out_convertible<typename std::remove_reference<decltype(*iter)>::type>;
+
+// Helper function that automatically infers 'T'.
+// To use when the argument is already a pointer.
+//
+// ptr must be not-null, else a DCHECK failure will occur.
+//
+// Returns a type that is implicitly convertible to either out<T> or T* depending
+// on the call site.
+//
+// Example:
+// int do_work(size_t args, out<int> result);
+// // returns 0 on success, sets result, otherwise error code
+//
+// Usage:
+// int* array = &some_other_value;
+// if (do_work(3, outof_ptr(array))) {
+// cout << "success: " << *array;
+// }
+template <typename T>
+out_convertible<T> outof_ptr(T* ptr) ALWAYS_INLINE;
+
+// Zero-overhead wrapper around a non-null non-const pointer meant to be used to output
+// the result of parameters. There are no other extra guarantees.
+//
+// The most common use case is to treat this like a typical pointer argument, for example:
+//
+// void write_out_5(out<int> x) {
+// *x = 5;
+// }
+//
+// The following operations are supported:
+// operator* -> use like a pointer (guaranteed to be non-null)
+// == and != -> compare against other pointers for (in)equality
+// begin/end -> use in standard C++ algorithms as if it was an iterator
+template <typename T>
+struct out {
+ // Has to be mutable lref. Otherwise how would you write something as output into it?
+ explicit inline out(T& param)
+ : param_(param) {}
+
+ // Model a single-element iterator (or pointer) to the parameter.
+ inline T& operator *() {
+ return param_;
+ }
+
+ //
+ // Comparison against this or other pointers.
+ //
+ template <typename T2>
+ inline bool operator==(const T2* other) const {
+ return std::addressof(param_) == other;
+ }
+
+ template <typename T2>
+ inline bool operator==(const out<T>& other) const {
+ return std::addressof(param_) == std::addressof(other.param_);
+ }
+
+ // An out-parameter is never null.
+ inline bool operator==(std::nullptr_t) const {
+ return false;
+ }
+
+ template <typename T2>
+ inline bool operator!=(const T2* other) const {
+ return std::addressof(param_) != other;
+ }
+
+ template <typename T2>
+ inline bool operator!=(const out<T>& other) const {
+ return std::addressof(param_) != std::addressof(other.param_);
+ }
+
+ // An out-parameter is never null.
+ inline bool operator!=(std::nullptr_t) const {
+ return true;
+ }
+
+ //
+ // Iterator interface implementation. Use with standard algorithms.
+ // TODO: (add items in iterator_traits if this is truly useful).
+ //
+
+ inline T* begin() {
+ return std::addressof(param_);
+ }
+
+ inline const T* begin() const {
+ return std::addressof(param_);
+ }
+
+ inline T* end() {
+ return std::addressof(param_) + 1;
+ }
+
+ inline const T* end() const {
+ return std::addressof(param_) + 1;
+ }
+
+ private:
+ T& param_;
+};
+
+//
+// IMPLEMENTATION DETAILS
+//
+
+//
+// This intermediate type should not be used directly by user code.
+//
+// It enables 'outof(x)' to be passed into functions that expect either
+// an out<T> **or** a regular C-style pointer (T*).
+//
+template <typename T>
+struct out_convertible {
+ explicit inline out_convertible(T& param)
+ : param_(param) {
+ }
+
+ // Implicitly convert into an out<T> for standard usage.
+ inline operator out<T>() {
+ return out<T>(param_);
+ }
+
+ // Implicitly convert into a '*' for legacy usage.
+ inline operator T*() {
+ return std::addressof(param_);
+ }
+ private:
+ T& param_;
+};
+
+// Helper function that automatically infers 'T'
+template <typename T>
+inline out_convertible<T> outof(T& param) {
+ return out_convertible<T>(param);
+}
+
+// Helper function that automatically infers 'T'.
+// To use when the argument is already inside an iterator.
+template <typename It>
+inline auto outof_iterator(It iter)
+ -> out_convertible<typename std::remove_reference<decltype(*iter)>::type> {
+ return outof(*iter);
+}
+
+// Helper function that automatically infers 'T'.
+// To use when the argument is already a pointer.
+template <typename T>
+inline out_convertible<T> outof_ptr(T* ptr) {
+ DCHECK(ptr != nullptr);
+ return outof(*ptr);
+}
+
+// Helper function that automatically infers 'T'.
+// Forwards an out parameter from one function into another.
+template <typename T>
+inline out_convertible<T> outof_forward(out<T>& out_param) {
+ T& param = std::addressof(*out_param);
+ return out_convertible<T>(param);
+}
+
+} // namespace art
+#endif // ART_RUNTIME_BASE_OUT_H_
diff --git a/runtime/base/out_test.cc b/runtime/base/out_test.cc
new file mode 100644
index 0000000..4274200
--- /dev/null
+++ b/runtime/base/out_test.cc
@@ -0,0 +1,99 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "out.h"
+
+#include <algorithm>
+#include <gtest/gtest.h>
+
+namespace art {
+
+struct OutTest : public testing::Test {
+ // Multiplies values less than 10 by two, stores the result and returns 0.
+ // Returns -1 if the original value was not multiplied by two.
+ static int multiply_small_values_by_two(size_t args, out<int> result) {
+ if (args < 10) {
+ *result = args * 2;
+ return 0;
+ } else {
+ return -1;
+ }
+ }
+};
+
+extern "C" int multiply_small_values_by_two_legacy(size_t args, int* result) {
+ if (args < 10) {
+ *result = args * 2;
+ return 0;
+ } else {
+ return -1;
+ }
+}
+
+TEST_F(OutTest, TraditionalCall) {
+ // For calling traditional C++ functions.
+ int res;
+ EXPECT_EQ(multiply_small_values_by_two(1, outof(res)), 0);
+ EXPECT_EQ(2, res);
+}
+
+TEST_F(OutTest, LegacyCall) {
+ // For calling legacy, e.g. C-style functions.
+ int res2;
+ EXPECT_EQ(0, multiply_small_values_by_two_legacy(1, outof(res2)));
+ EXPECT_EQ(2, res2);
+}
+
+TEST_F(OutTest, CallFromIterator) {
+ // For calling a function with a parameter originating as an iterator.
+ std::vector<int> list = {1, 2, 3}; // NOLINT [whitespace/labels] [4]
+ std::vector<int>::iterator it = list.begin();
+
+ EXPECT_EQ(0, multiply_small_values_by_two(2, outof_iterator(it)));
+ EXPECT_EQ(4, list[0]);
+}
+
+TEST_F(OutTest, CallFromPointer) {
+ // For calling a function with a parameter originating as a C-pointer.
+ std::vector<int> list = {1, 2, 3}; // NOLINT [whitespace/labels] [4]
+
+ int* list_ptr = &list[2]; // 3
+
+ EXPECT_EQ(0, multiply_small_values_by_two(2, outof_ptr(list_ptr)));
+ EXPECT_EQ(4, list[2]);
+}
+
+TEST_F(OutTest, OutAsIterator) {
+ // For using the out<T> parameter as an iterator inside of the callee.
+ std::vector<int> list;
+ int x = 100;
+ out<int> out_from_x = outof(x);
+
+ for (const int& val : out_from_x) {
+ list.push_back(val);
+ }
+
+ ASSERT_EQ(1u, list.size());
+ EXPECT_EQ(100, list[0]);
+
+ // A more typical use-case would be to use std algorithms
+ EXPECT_NE(out_from_x.end(),
+ std::find(out_from_x.begin(),
+ out_from_x.end(),
+ 100)); // Search for '100' in out.
+}
+
+} // namespace art
diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h
index 2486a98..a6cccef 100644
--- a/runtime/interpreter/interpreter_common.h
+++ b/runtime/interpreter/interpreter_common.h
@@ -553,7 +553,7 @@
ArtMethod* unboxed_closure = nullptr;
// Raise an exception if unboxing fails.
if (!Runtime::Current()->GetLambdaBoxTable()->UnboxLambda(boxed_closure_object,
- &unboxed_closure)) {
+ outof(unboxed_closure))) {
CHECK(self->IsExceptionPending());
return false;
}
diff --git a/runtime/lambda/box_table.cc b/runtime/lambda/box_table.cc
index 64a6076..22cc820 100644
--- a/runtime/lambda/box_table.cc
+++ b/runtime/lambda/box_table.cc
@@ -94,8 +94,7 @@
return method_as_object;
}
-bool BoxTable::UnboxLambda(mirror::Object* object, ClosureType* out_closure) {
- DCHECK(object != nullptr);
+bool BoxTable::UnboxLambda(mirror::Object* object, out<ClosureType> out_closure) {
*out_closure = nullptr;
// Note that we do not need to access lambda_table_lock_ here
diff --git a/runtime/lambda/box_table.h b/runtime/lambda/box_table.h
index 312d811..c6d3d0c 100644
--- a/runtime/lambda/box_table.h
+++ b/runtime/lambda/box_table.h
@@ -18,6 +18,7 @@
#include "base/allocator.h"
#include "base/hash_map.h"
+#include "base/out.h"
#include "gc_root.h"
#include "base/macros.h"
#include "base/mutex.h"
@@ -51,7 +52,7 @@
SHARED_REQUIRES(Locks::mutator_lock_) REQUIRES(!Locks::lambda_table_lock_);
// Unboxes an object back into the lambda. Returns false and throws an exception on failure.
- bool UnboxLambda(mirror::Object* object, ClosureType* out_closure)
+ bool UnboxLambda(mirror::Object* object, out<ClosureType> out_closure)
SHARED_REQUIRES(Locks::mutator_lock_);
// Sweep weak references to lambda boxes. Update the addresses if the objects have been
diff --git a/runtime/stack.cc b/runtime/stack.cc
index b07b244..2916eaa 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -19,6 +19,7 @@
#include "arch/context.h"
#include "art_method-inl.h"
#include "base/hex_dump.h"
+#include "base/out.h"
#include "entrypoints/entrypoint_utils-inl.h"
#include "entrypoints/runtime_asm_entrypoints.h"
#include "gc_map.h"
@@ -180,7 +181,7 @@
} else {
uint16_t reg = code_item->registers_size_ - code_item->ins_size_;
uint32_t value = 0;
- bool success = GetVReg(m, reg, kReferenceVReg, &value);
+ bool success = GetVReg(m, reg, kReferenceVReg, outof(value));
// We currently always guarantee the `this` object is live throughout the method.
CHECK(success) << "Failed to read the this object in " << PrettyMethod(m);
return reinterpret_cast<mirror::Object*>(value);
@@ -375,8 +376,8 @@
QuickMethodFrameInfo frame_info = m->GetQuickFrameInfo(code_pointer);
uint32_t vmap_offset_lo, vmap_offset_hi;
// TODO: IsInContext stops before spotting floating point registers.
- if (vmap_table.IsInContext(vreg, kind_lo, &vmap_offset_lo) &&
- vmap_table.IsInContext(vreg + 1, kind_hi, &vmap_offset_hi)) {
+ if (vmap_table.IsInContext(vreg, kind_lo, outof(vmap_offset_lo)) &&
+ vmap_table.IsInContext(vreg + 1, kind_hi, outof(vmap_offset_hi))) {
bool is_float = (kind_lo == kDoubleLoVReg);
uint32_t spill_mask = is_float ? frame_info.FpSpillMask() : frame_info.CoreSpillMask();
uint32_t reg_lo = vmap_table.ComputeRegister(spill_mask, vmap_offset_lo, kind_lo);
@@ -399,8 +400,8 @@
uint64_t* val) const {
uint32_t low_32bits;
uint32_t high_32bits;
- bool success = GetVRegFromOptimizedCode(m, vreg, kind_lo, &low_32bits);
- success &= GetVRegFromOptimizedCode(m, vreg + 1, kind_hi, &high_32bits);
+ bool success = GetVRegFromOptimizedCode(m, vreg, kind_lo, outof(low_32bits));
+ success &= GetVRegFromOptimizedCode(m, vreg + 1, kind_hi, outof(high_32bits));
if (success) {
*val = (static_cast<uint64_t>(high_32bits) << 32) | static_cast<uint64_t>(low_32bits);
}
@@ -452,7 +453,7 @@
QuickMethodFrameInfo frame_info = m->GetQuickFrameInfo(code_pointer);
uint32_t vmap_offset;
// TODO: IsInContext stops before spotting floating point registers.
- if (vmap_table.IsInContext(vreg, kind, &vmap_offset)) {
+ if (vmap_table.IsInContext(vreg, kind, outof(vmap_offset))) {
bool is_float = (kind == kFloatVReg) || (kind == kDoubleLoVReg) || (kind == kDoubleHiVReg);
uint32_t spill_mask = is_float ? frame_info.FpSpillMask() : frame_info.CoreSpillMask();
uint32_t reg = vmap_table.ComputeRegister(spill_mask, vmap_offset, kind);
@@ -532,8 +533,8 @@
QuickMethodFrameInfo frame_info = m->GetQuickFrameInfo(code_pointer);
uint32_t vmap_offset_lo, vmap_offset_hi;
// TODO: IsInContext stops before spotting floating point registers.
- if (vmap_table.IsInContext(vreg, kind_lo, &vmap_offset_lo) &&
- vmap_table.IsInContext(vreg + 1, kind_hi, &vmap_offset_hi)) {
+ if (vmap_table.IsInContext(vreg, kind_lo, outof(vmap_offset_lo)) &&
+ vmap_table.IsInContext(vreg + 1, kind_hi, outof(vmap_offset_hi))) {
bool is_float = (kind_lo == kDoubleLoVReg);
uint32_t spill_mask = is_float ? frame_info.FpSpillMask() : frame_info.CoreSpillMask();
uint32_t reg_lo = vmap_table.ComputeRegister(spill_mask, vmap_offset_lo, kind_lo);
