Merged PR 2055: Remove using namespace from headers under lotus/core

1. Remove using namespace from headers under lotus/core
2. Fix a tiny bug in ExecutionFrame::ReleaseMLValue function

Related work items: #137

Author: snnn

lotus/core/common/common.h

@@ -22,7 +22,6 @@
 
 #include <algorithm>
 #include <functional>
-#include <map>
 #include <memory>
 #include <numeric>
 #include <set>
@@ -39,21 +38,11 @@
 
 namespace Lotus {
 
-template <typename Key, typename Value>
-using LotusMap = std::unordered_map<Key, Value>;
-
 using TimePoint = std::chrono::high_resolution_clock::time_point;
 
 // Using statements for common classes that we refer to in lotus very often.
 // TODO(Task:137) Remove 'using' statements from header files
 using Common::Status;
-using Common::StatusCategory;
-using Common::StatusCode;
-using std::make_unique;
-using std::set;
-using std::string;
-using std::unique_ptr;
-using std::vector;
 
 #ifdef _WIN32
 #define UNUSED_PARAMETER(x) (x)
@@ -91,7 +80,7 @@ std::vector<std::string> GetStackTrace();
   if (!(condition)) throw Lotus::LotusException(WHERE_WITH_STACK, #condition, Lotus::MakeString(__VA_ARGS__))
 
 #define LOTUS_MAKE_STATUS(category, code, ...) \
-  Status(category, code, Lotus::MakeString(__VA_ARGS__))
+  Lotus::Common::Status(Lotus::Common::category, Lotus::Common::code, Lotus::MakeString(__VA_ARGS__))
 
 // Macros to disable the copy and/or move ctor and assignment methods
 // These are usually placed in the private: declarations for a class.
@@ -143,33 +132,33 @@ std::vector<std::string> GetStackTrace();
 #define LOTUS_EXPORT
 #endif
 
-inline void MakeStringInternal(std::stringstream& /*ss*/) noexcept {}
+inline void MakeStringInternal(std::ostringstream& /*ss*/) noexcept {}
 
 template <typename T>
-inline void MakeStringInternal(std::stringstream& ss, const T& t) noexcept {
+inline void MakeStringInternal(std::ostringstream& ss, const T& t) noexcept {
   ss << t;
 }
 
 template <typename T, typename... Args>
-inline void MakeStringInternal(std::stringstream& ss, const T& t, const Args&... args) noexcept {
+inline void MakeStringInternal(std::ostringstream& ss, const T& t, const Args&... args) noexcept {
   ::Lotus::MakeStringInternal(ss, t);
   ::Lotus::MakeStringInternal(ss, args...);
 }
 
 template <typename... Args>
-string MakeString(const Args&... args) {
-  std::stringstream ss;
+std::string MakeString(const Args&... args) {
+  std::ostringstream ss;
   ::Lotus::MakeStringInternal(ss, args...);
-  return string(ss.str());
+  return std::string(ss.str());
 }
 
 // Specializations for already-a-string types.
 template <>
-inline string MakeString(const string& str) {
+inline std::string MakeString(const std::string& str) {
   return str;
 }
-inline string MakeString(const char* p_str) {
-  return string(p_str);
+inline std::string MakeString(const char* p_str) {
+  return p_str;
 }
 
 inline long long TimeDiffMicroSeconds(TimePoint start_time) {

lotus/core/common/logging/logging.cc

@@ -120,7 +120,7 @@ LoggingManager::~LoggingManager() {
 void LoggingManager::CreateDefaultLogger(const std::string &logger_id) {
   // this method is only called from ctor in scope where DefaultLoggerMutex() is already locked
 
-  unique_ptr<Logger> &default_logger{GetDefaultLogger()};
+  std::unique_ptr<Logger> &default_logger{GetDefaultLogger()};
 
   if (default_logger != nullptr) {
     throw std::logic_error("Default logger already set. ");

lotus/core/common/logging/logging.h

@@ -54,7 +54,7 @@ using Timestamp = std::chrono::time_point<std::chrono::system_clock>;
 #ifdef _DEBUG
 static bool vlog_enabled = true;  // Set directly based on your needs.
 #else
-static const bool vlog_enabled = false;  // no VLOG output
+constexpr bool vlog_enabled = false;  // no VLOG output
 #endif
 
 enum class DataType {
@@ -233,7 +233,7 @@ class Logger {
 
 inline const Logger &LoggingManager::DefaultLogger() {
   // fetch the container for the default logger once to void function calls in the future
-  static unique_ptr<Logger> &default_logger = GetDefaultLogger();
+  static std::unique_ptr<Logger> &default_logger = GetDefaultLogger();
 
   if (default_logger == nullptr) {
     // fail early for attempted misuse. don't use logging macros as we have no logger.

lotus/core/common/status.cc

@@ -19,11 +19,11 @@ bool Status::IsOK() const noexcept {
 }
 
 StatusCategory Status::Category() const noexcept {
-  return IsOK() ? StatusCategory::NONE : state_->category;
+  return IsOK() ? Common::NONE : state_->category;
 }
 
 int Status::Code() const noexcept {
-  return IsOK() ? static_cast<int>(StatusCode::OK) : state_->code;
+  return IsOK() ? static_cast<int>(Common::OK) : state_->code;
 }
 
 const std::string& Status::ErrorMessage() const noexcept {
@@ -37,11 +37,11 @@ std::string Status::ToString() const {
 
   std::string result;
 
-  if (StatusCategory::SYSTEM == state_->category) {
+  if (Common::SYSTEM == state_->category) {
     result += "SystemError";
     result += " : ";
     result += std::to_string(errno);
-  } else if (StatusCategory::LOTUS == state_->category) {
+  } else if (Common::LOTUS == state_->category) {
     result += "[LotusError]";
     result += " : ";
     result += std::to_string(Code());

lotus/core/common/status.h

@@ -83,7 +83,7 @@ class Status {
     State(StatusCategory cat0, int code0, const std::string& msg0)
         : category(cat0), code(code0), msg(msg0) {}
 
-    StatusCategory category = StatusCategory::NONE;
+    StatusCategory category = Common::NONE;
     int code = 0;
     std::string msg;
   };

lotus/core/framework/allocation_planner.cc

@@ -9,7 +9,7 @@
 #include "core/graph/utils.h"
 #include "core/framework/data_types.h"
 #include "core/framework/mldata_type_utils.h"
-
+using namespace Lotus::Common;
 using namespace onnx;
 namespace Lotus {
 
@@ -39,7 +39,7 @@ std::ostream& operator<<(std::ostream& out, std::pair<const SequentialExecutionP
   const SequentialExecutionPlan& plan = *planinfo.first;
   const SessionState& session_state = *planinfo.second;
   const LotusIR::Graph& graph = *session_state.GetGraph();
-  std::unordered_map<int, string> index_to_name;
+  std::unordered_map<int, std::string> index_to_name;
 
   out << "Allocation Plan:\n";
   for (auto& name_index : session_state.GetMLValueIdxMap()) {

lotus/core/framework/bfc_arena.h

@@ -52,7 +52,7 @@ struct AllocatorStats {
     this->total_allocated_bytes_ = 0;
   }
 
-  string DebugString() const {
+  std::string DebugString() const {
     std::stringstream ss;
     ss << "Limit:          " << this->bytes_limit << "\n"
        << "InUse:         " << this->bytes_in_use << "\n"
@@ -160,8 +160,8 @@ class BFCArena : public IArenaAllocator {
 
     bool in_use() const { return allocation_id != -1; }
 
-    string DebugString(BFCArena* a,
-                       bool recurse) {
+    std::string DebugString(BFCArena* a,
+                            bool recurse) {
       std::stringstream ss;
       ss << "  Size: " << size << " | Requested Size: " << requested_size << " | in_use: " << in_use();
       if (recurse && prev != BFCArena::kInvalidChunkHandle) {

lotus/core/framework/environment.cc

@@ -25,9 +25,9 @@ Status Environment::Initialize() {
     // TODO: Should register microsoft domain kernels here.
 
   } catch (std::exception& ex) {
-    status = Status{LOTUS, StatusCode::RUNTIME_EXCEPTION, std::string{"Exception caught: "} + ex.what()};
+    status = Status{LOTUS, Common::RUNTIME_EXCEPTION, std::string{"Exception caught: "} + ex.what()};
   } catch (...) {
-    status = Status{LOTUS, StatusCode::RUNTIME_EXCEPTION};
+    status = Status{LOTUS, Common::RUNTIME_EXCEPTION};
   }
 
   return status;

lotus/core/framework/execution_frame.cc

@@ -3,7 +3,7 @@
 #include "core/framework/op_kernel.h"
 #include "core/framework/session_state.h"
 #include "core/framework/mem_pattern_planner.h"
-
+using namespace Lotus::Common;
 namespace Lotus {
 
 ExecutionFrame::ExecutionFrame(const std::unordered_map<std::string, MLValue>& feeds,
@@ -74,7 +74,7 @@ Status ExecutionFrame::AllocateMLValueTensorSelfOwnBufferHelper(int mlvalue_inde
 
   int64_t len = shape.Size();
   if (len < 0) {
-    return Status(StatusCategory::LOTUS, StatusCode::INVALID_ARGUMENT, "Tensor shape cannot contain any negative value");
+    return Status(Common::LOTUS, Common::INVALID_ARGUMENT, "Tensor shape cannot contain any negative value");
   }
   len *= element_type->Size();
   //safety check for 32 bits systems
@@ -288,8 +288,8 @@ Common::Status ExecutionFrame::AllocateAsPerAllocationPlan(int mlvalue_index,
 }
 
 void ExecutionFrame::Init(const LotusIR::Graph* graph,
-                          const std::unordered_map<string, MLValue>& feeds,
-                          const std::vector<string>& output_names,
+                          const std::unordered_map<std::string, MLValue>& feeds,
+                          const std::vector<std::string>& output_names,
                           const std::vector<MLValue>& fetches) {
   LOTUS_ENFORCE(graph);
 

lotus/core/framework/execution_frame.h

@@ -11,7 +11,6 @@
 #include "core/framework/allocation_planner.h"
 #include "core/framework/ml_value_patterns_planner.h"
 #include "core/common/logging/logging.h"
-// #include "core/framework/session_state.h"
 
 namespace Lotus {
 
@@ -115,34 +114,37 @@ class ExecutionFrame {
   // Return nullptr if index map to an value that is an unused optional input/output
   template <typename T>
   const T* GetValue(int index) const {
-    LOTUS_ENFORCE(index >= 0 && index < node_values_.size());
+    LOTUS_ENFORCE(index >= 0 && static_cast<size_t>(index) < node_values_.size());
     return node_values_[index] >= 0 ? &all_values_[node_values_[index]].Get<T>() : nullptr;
   }
 
   Fence_t GetFence(int index) const {
-    LOTUS_ENFORCE(index >= 0 && index < node_values_.size());
+    LOTUS_ENFORCE(index >= 0 && static_cast<size_t>(index) < node_values_.size());
     return node_values_[index] >= 0 ? all_values_[node_values_[index]].Fence() : nullptr;
   }
 
   // Return nullptr if index map to an value that is an unused optional input/output
   MLDataType GetType(int index) const {
-    LOTUS_ENFORCE(index >= 0 && index < node_values_.size());
+    LOTUS_ENFORCE(index >= 0 && static_cast<size_t>(index) < node_values_.size());
     return node_values_[index] >= 0 ? all_values_[node_values_[index]].Type() : nullptr;
   }
 
   // Return nullptr if index map to an value that is an unused optional input/output
   template <typename T>
   T* GetMutableValue(int index) {
-    LOTUS_ENFORCE(index >= 0 && index < node_values_.size());
+    LOTUS_ENFORCE(index >= 0 && static_cast<size_t>(index) < node_values_.size());
     return node_values_[index] >= 0 ? all_values_[node_values_[index]].GetMutable<T>() : nullptr;
   }
 
   AllocatorPtr GetAllocator(const AllocatorInfo& info);
 
-  void ReleaseMLValue(int mlvalue_idx) {
-    LOTUS_ENFORCE(mlvalue_idx >= 0 || mlvalue_idx < all_values_.size());
+  Status ReleaseMLValue(int mlvalue_idx) {
+    if (mlvalue_idx < 0 || mlvalue_idx >= all_values_.size()) {
+      return LOTUS_MAKE_STATUS(LOTUS, INVALID_ARGUMENT, "invalid index ", mlvalue_idx);
+    }
     all_values_[mlvalue_idx] = MLValue();
     TraceFree(mlvalue_idx);
+    return Status::OK();
   }
 
   const Lotus::SessionState& SessionState() const {
@@ -170,8 +172,8 @@ class ExecutionFrame {
                                                   bool create_fence);
 
   void Init(const LotusIR::Graph* graph,
-            const std::unordered_map<string, MLValue>& feeds,
-            const std::vector<string>& output_names,
+            const std::unordered_map<std::string, MLValue>& feeds,
+            const std::vector<std::string>& output_names,
             const std::vector<MLValue>& fetches);
 
   void SetupNodeArg(const LotusIR::NodeArg* arg);
@@ -187,7 +189,7 @@ class ExecutionFrame {
   template <typename T>
   Status GetOrCreateMLValue(int index, const MLValueAllocationParameters& parameters, T*& value) {
     if (index < 0 || index >= node_values_.size()) {
-      return Status(LOTUS, INVALID_ARGUMENT,
+      return Status(Common::LOTUS, Common::INVALID_ARGUMENT,
                     "Try to access with invalid node value index: " + std::to_string(index));
     }
 
@@ -236,7 +238,7 @@ class ExecutionFrame {
   // i-th kernel is still waiting for pending_counts_[i] inputs.
   std::vector<int> pending_counts_;  // not used currently
 
-  std::unordered_map<string, int> value_name_to_index_;
+  std::unordered_map<std::string, int> value_name_to_index_;
 
   const Lotus::SessionState& session_state_;