CR: A clean look at the code, minor tweaks.

Author: dkorolev

Blocks/HTTP/docu/server/docu_03httpserver_04_test.cc

@@ -83,7 +83,7 @@ const auto port = FLAGS_docu_net_server_port_04;
       }
     } catch (const current::Exception& e) {
       // TODO(dkorolev): Catch the right exception type.
-      r(PennyOutput{e.DetailedDescription(), 0});
+      r(PennyOutput{e.what(), 0});
     }
   });
 EXPECT_EQ("{\"error\":\"\",\"result\":5}\n", HTTP(POST(Printf("http://localhost:%d/penny", port), PennyInput{"add",{2,3}})).body);

Blocks/README.md

@@ -91,7 +91,7 @@ const auto scope = HTTP(port).Register("/penny", [](Request r) {
     }
   } catch (const current::Exception& e) {
     // TODO(dkorolev): Catch the right exception type.
-    r(PennyOutput{e.DetailedDescription(), 0});
+    r(PennyOutput{e.what(), 0});
   }
 });
 ```

Bricks/exception.h

@@ -76,11 +76,12 @@ class Exception : public std::exception {
 };
 
 // Extra parenthesis around `e((E))` are essential to not make it a function declaration.
-#define CURRENT_THROW(E)                   \
-  {                                        \
-    auto e((E));                           \
-    e.FillDetails(#E, __FILE__, __LINE__); \
-    throw e;                               \
+// Also, call it `_e_` to avoid name collisions.
+#define CURRENT_THROW(E)                     \
+  {                                          \
+    auto _e_((E));                           \
+    _e_.FillDetails(#E, __FILE__, __LINE__); \
+    throw _e_;                               \
   }
 
 }  // namespace current

Bricks/sync/test.cc

@@ -255,8 +255,8 @@ TEST(WaitableAtomic, Smoke) {
   EXPECT_TRUE(copy_of_object.x_done);
   EXPECT_TRUE(copy_of_object.y_done);
 
-  // Both threads should have had enough time to increment their counters at least by a bit.
-  // Technically, the EXPECT-s below make the test flaky, but the range is generous enough.
+// Both threads should have had enough time to increment their counters at least by a bit.
+// Technically, the EXPECT-s below make the test flaky, but the range is generous enough.
 #ifndef CURRENT_COVERAGE_REPORT_MODE
   EXPECT_GT(copy_of_object.x, 10u);
   EXPECT_LT(copy_of_object.x, 100u);

Integrations/OneSignal/ios_notifications_sender.h

@@ -109,8 +109,7 @@ server {
   // Send one iOS push notification.
   // Returns true on success.
   // Returns false on "regular" errors.
-  // Throws on terrible errors, with the returned `e.DetailedDescription()` good for journaling and further
-  // investigation.
+  // Throws on terrible errors, with the returned `e.DetailedDescription()` good for journaling.
   bool Push(const std::string& recipient_player_id,
             const std::string& message = "",
             int32_t increase_counter = 0) const {
@@ -147,23 +146,23 @@ server {
           ParseJSON(response.body, parsed_response);
         } catch (const Exception& e) {
           // Resulting JSON error: Fatal.
-          throw OneSignalPushNotificationException("OneSignal iOS push error A: " + e.DetailedDescription());
+          CURRENT_THROW(OneSignalPushNotificationException("OneSignal iOS push error A: " + e.DetailedDescription()));
         }
         if (!Exists(parsed_response.recipients)) {
           if (Exists(parsed_response.errors)) {
             // Some "regular" error occurred: Non-fatal.
             return false;
           } else {
             // Some unexpected error occurred: Fatal.
-            throw OneSignalPushNotificationException("OneSignal iOS push error Q: " + response.body);
+            CURRENT_THROW(OneSignalPushNotificationException("OneSignal iOS push error Q: " + response.body));
           }
         }
         if (Value(parsed_response.recipients) == 0) {
           // Just could not send the notification: Non-fatal.
           return false;
         } else if (Value(parsed_response.recipients) > 1) {
           // Potentially sent the notification to more than one user: Fatal.
-          throw OneSignalPushNotificationException("OneSignal iOS push error Z: " + response.body);
+          CURRENT_THROW(OneSignalPushNotificationException("OneSignal iOS push error Z: " + response.body));
         } else {
           // The notification has been sent to exactly one user: OK.
           return true;

Storage/api.h

@@ -400,6 +400,7 @@ class RESTfulStorage {
   using mutable_fields_t = MutableFields<STORAGE_IMPL>;
 
   // TODO(dkorolev): `unique_ptr` and move semantics. Move to `-std=c++14` maybe? :-)
+  // FIXME_DIMA <-- keep this marker for me to find it once we do move to C++14.
   using cqs_universal_parser_t = std::function<std::shared_ptr<CurrentStruct>(Request&)>;
   using cqs_query_handler_t = std::function<Response(
       immutable_fields_t, std::shared_ptr<CurrentStruct> command, const std::string& restful_url_prefix)>;

Storage/rest/types.h

@@ -326,6 +326,7 @@ CURRENT_STRUCT(CQSUserCodeError, generic::RESTGenericResponse) {
     map_t map;
     map["error"] = e.OriginalDescription();
 #ifndef NDEBUG
+    // As discussed between @dkorolev and @mzhurovich, only return `caller`, `file`, and `line` in non-`NDEBUG` builds.
     map["caller"] = e.Caller();
     if (e.File()) {
       map["file"] = e.File();

TypeSystem/Reflection/test.cc

@@ -327,18 +327,6 @@ struct CollectFieldValues {
   }
 };
 
-CURRENT_STRUCT(StructWithSizeTWhichShouldBeUInt64T) {
-  CURRENT_FIELD(a, uint64_t);
-  CURRENT_FIELD(b, size_t);
-};
-
-static_assert(std::is_same<decltype(std::declval<StructWithSizeTWhichShouldBeUInt64T>().a), uint64_t>::value, "");
-static_assert(std::is_same<decltype(std::declval<StructWithSizeTWhichShouldBeUInt64T>().b), uint64_t>::value, "");
-
-static_assert(std::is_same<decltype(std::declval<StructWithSizeTWhichShouldBeUInt64T>().a),
-                           decltype(std::declval<StructWithSizeTWhichShouldBeUInt64T>().b)>::value,
-              "");
-
 }  // namespace reflection_test
 
 TEST(Reflection, VisitAllFieldsWithoutObject) {

TypeSystem/struct.h

@@ -96,13 +96,6 @@ struct FieldImpl<DF, T> {
   typedef T type;
 };
 
-// Allow `size_t` in JSON-serializable types, but make it `uint64_t` right way.
-// Apparently, this is important when compiling on OS X, where `size_t` and `uint64_t` are different types. -- D.K.
-template <>
-struct FieldImpl<DF, size_t> {
-  typedef uint64_t type;
-};
-
 template <typename T>
 struct FieldImpl<FC, T> {
   // TODO: Read on padding.

scripts/full-test.sh

@@ -64,12 +64,11 @@ done
 echo -e -n "\033[0m"
 
 (
-  # Compile and run The Big Test.
+  # Build and run The Big Test.
+
   ln -sf "$CURRENT_SCRIPTS_DIR/MakefileWithCurrentBuild" "$FULL_TEST_DIR_NAME/Makefile"
   (cd "$FULL_TEST_DIR_NAME" ; make phony_current_build > /dev/null ; unlink Makefile)
-)
 
-(
   cd "$FULL_TEST_DIR_NAME"
 
   echo -e "\033[0m"