> On Feb. 25, 2016, 10:09 p.m., Andrew Stitcher wrote: > > proton-c/bindings/cpp/include/proton/types.hpp, line 31 > > <https://reviews.apache.org/r/43742/diff/2/?file=1264533#file1264533line31> > > > > What does the XXX comment here mean? It doesn't seem relevant to this > > file, can we just remove it?
I added that comment. It's a minor consistency question. In every other instance we use the style 'include "protonheader.h"'. Is this different for a reason? - Justin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43742/#review120777 ----------------------------------------------------------- On Feb. 25, 2016, 9:31 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43742/ > ----------------------------------------------------------- > > (Updated Feb. 25, 2016, 9:31 p.m.) > > > Review request for qpid, Andrew Stitcher, Cliff Jansen, and Justin Ross. > > > Bugs: PROTON-1138 > https://issues.apache.org/jira/browse/PROTON-1138 > > > Repository: qpid-proton-git > > > Description > ------- > > PROTON-1138: Add proton::uuid type. > > Official C++ representation of a UUID, removed amqp_uuid. > > Also added proton::byte_array base class for this and other fixed-size proton > types that lack C++ native equivalents so need to be presented in C++ as > byte-arrays. > > PROTON-1138: Add proton::timestamp type. > > - Removed amqp_timestamp. > - Got rid of public duration::milliseconds data member. > - Made duration and timestamp consistent with const int64_t ms() accessor, > explicit constructor and op=(int64_t) > - Completed duration/timestamp arithmetic. > > NOTE changed duration to use signed int64_t. Durations can be added and > subtracted and negative durations can occur during calculations. Also AMQP > timestamp is defined as signed so we should be consistent. > > UNIX time_t is traditionally unsigned because signed 32 bits is too small to > express a useful time range after the epoch (frankly unsigned is too, watch > out > in 2038 :) 64 bit timestamps and durations do not require this restriction. > > PROTON-1138: Add proton::decimal types. > > > PROTON-1138: Simplify comparable class, make comparable inheritance private. > > Simplified the comparable class. > > - comparable<T> is a private empty base class that exists only to instantiate > operator friends of T. > - comparable<T> is never used in any other way. > - for any compiler with even rudimentary empty-base-class support, this has 0 > run-time overhead. > > I did some research to try and eliminate inheritance entirely. There are 2 > well > known approaches. > > 1. boost::operators uses exactly the approach above. > 2. std::rel_ops uses unconstrained templates that match anything with op == > or < operator, we already agreed that approach is unsuitable for public API. > > I tried type-trait approaches but could not find one that was better than the > above. IMO inheritance is the right fit: It has no run-time overhead, it is > easy to understand, and we *want* "is-a" inheritance semantics: if `object` is > comparable then sub-classes of `object` should be comparable. (That is the > part > that is hard to achieve with type-traits. When I realized I was trying to > reinvent inheritance I decided to let go.) > > PROTON-1138: c++: Extend_CXX_FLAGS in CMakeLists.txt, don't overwrite user > setting. > > > Diffs > ----- > > proton-c/CMakeLists.txt 862a520f5103cdd5a08539fd3bfc9b49eeabdef5 > proton-c/bindings/cpp/CMakeLists.txt > 7c1c41dfa1f235864c23a459209ed024b81fa7cd > proton-c/bindings/cpp/include/proton/byte_array.hpp PRE-CREATION > proton-c/bindings/cpp/include/proton/comparable.hpp > 6c6424963929ae90765e84ede0bbb32fd15feb85 > proton-c/bindings/cpp/include/proton/decimal.hpp PRE-CREATION > proton-c/bindings/cpp/include/proton/decoder.hpp > 3dab9c94f4875a9a8ef4323f30ee999fd01f8427 > proton-c/bindings/cpp/include/proton/duration.hpp > f2068494b213ac67034fab59364da1ea790af505 > proton-c/bindings/cpp/include/proton/encoder.hpp > 187641237d0b31f712f506a61b4c1db7214e0c17 > proton-c/bindings/cpp/include/proton/endpoint.hpp > 8a8712ed22c3dacdae4f21a65268a313a19db66b > proton-c/bindings/cpp/include/proton/message.hpp > 045143f5b22e0275d193f058bdfe850db472bac2 > proton-c/bindings/cpp/include/proton/message_id.hpp > 0adef07e45ccc03775decb8a4fb82c8f8fbe0cf3 > proton-c/bindings/cpp/include/proton/object.hpp > 4e12e6fe9c4800c5afe6e6b5fd01dc10f7a213f1 > proton-c/bindings/cpp/include/proton/reactor.hpp > 82514058ccb33820b6aa5635b303e3f5b32d6fbd > proton-c/bindings/cpp/include/proton/scalar.hpp > 3975301e8b1380d92d5a312c475cca6069ff86a2 > proton-c/bindings/cpp/include/proton/timestamp.hpp PRE-CREATION > proton-c/bindings/cpp/include/proton/type_traits.hpp > 56f503ce37ef54cf6636dd4632e8846b4c0ae6e1 > proton-c/bindings/cpp/include/proton/types.hpp > ee93082965e896f736d60d894767b4109ecdee87 > proton-c/bindings/cpp/include/proton/url.hpp > ccf3f1ed389d080a1790069ce7a2c9e1029dfef1 > proton-c/bindings/cpp/include/proton/uuid.hpp PRE-CREATION > proton-c/bindings/cpp/include/proton/value.hpp > b2c1c0bb01350e0228c711a3d43fa1310d6e2c31 > proton-c/bindings/cpp/src/condition.cpp > 9220fd7aa672fb3a57482a0e30af65ee3fb7c691 > proton-c/bindings/cpp/src/connection_engine.cpp > 89d901e588d5dffb1e760509d475bf56c6df9bf3 > proton-c/bindings/cpp/src/connection_options.cpp > 2751aaa4f663dedf24d24b25b39da47fc7c50471 > proton-c/bindings/cpp/src/container_impl.cpp > 8527b68feb2294de966020762874708d5e3f9a6b > proton-c/bindings/cpp/src/data.cpp 6783dae23b93114110f8f6ef90b3fe993ea9a334 > proton-c/bindings/cpp/src/decoder.cpp > f775001c40f6da158ad2ef11022a4e24038d13ca > proton-c/bindings/cpp/src/duration.cpp > 8c8263e411f1f9b064bbee4d4924d4fb728b9354 > proton-c/bindings/cpp/src/encoder.cpp > 58b41b3745ef2224ca0bca40c1481d88fe0e2a8f > proton-c/bindings/cpp/src/engine_test.cpp > cb8e38908396f90f23441b7070a222e3fc907f27 > proton-c/bindings/cpp/src/message.cpp > 2358da23bec8a354aa5db63397acf59236488456 > proton-c/bindings/cpp/src/message_test.cpp > 988aa34891839088ad7f256c5263266339be8ec8 > proton-c/bindings/cpp/src/reactor.cpp > 59db5e082e06727128c283361e1967c6dbc38573 > proton-c/bindings/cpp/src/reconnect_timer.cpp > 2a44063bc41c98108a26155e5b4577d05b032b87 > proton-c/bindings/cpp/src/scalar.cpp > 3379c015c07e337cc35ef1ebfc32554b658d4607 > proton-c/bindings/cpp/src/scalar_test.cpp > a9700b0133830fa8b9e01fb1f761805a218bd8b6 > proton-c/bindings/cpp/src/types.cpp > 1ace1a17fdf46b8ad068c395267fe24dedc59913 > proton-c/bindings/cpp/src/types_internal.hpp PRE-CREATION > proton-c/bindings/cpp/src/uuid.hpp e8ee908e95e8eff0f1388fe8fe41218dc2d504c9 > proton-c/bindings/cpp/src/uuid.cpp 6e8367fc52de5cd776cd72faeb843c90279d61e2 > tests/tools/apps/cpp/reactor_send.cpp > 76fa9dcb4636315ee1ea760732946b9ac403c4b9 > > Diff: https://reviews.apache.org/r/43742/diff/ > > > Testing > ------- > > ctest -R cpp # not tested across c++ versions or platforms yet. > > > Thanks, > > Alan Conway > >
