Also there are 4 places where tabs were replaced by spaces, marked them below. >Вторник, 4 августа 2020, 12:55 +03:00 от Ilya Kosarev : >  >Hi! > >Yes, this patch seems to be nice. >  >See 1 comment below. >>Понедельник, 13 июля 2020, 17:18 +03:00 от Aleksandr Lyapunov < alyapunov@tarantool.org >: >>  >>We should use noexcept where it's appropriate. >>1.It informs a developer he could not care about exception safety >>2.It instructs a compiler to check throws if possible >> >>We should override for compile-time check of misprints in names >>of methods and types of arguments. >> >>Part of #5153 >>--- >> src/box/alter.cc | 180 +++++++++++++++++++++++++++---------------------------- >> 1 file changed, 90 insertions(+), 90 deletions(-) >> >>diff --git a/src/box/alter.cc b/src/box/alter.cc >>index 249cee2..15a51d0 100644 >>--- a/src/box/alter.cc >>+++ b/src/box/alter.cc >>@@ -765,7 +765,7 @@ struct alter_space; >>  >> class AlterSpaceOp { >> public: >>- AlterSpaceOp(struct alter_space *alter); >>+ explicit AlterSpaceOp(struct alter_space *alter); >>  >>  /** Link in alter_space::ops. */ >>  struct rlist link; >>@@ -774,7 +774,7 @@ public: >>  * the space definition and/or key list that will be used >>  * for creating the new space. Must not yield or fail. >>  */ >>- virtual void alter_def(struct alter_space * /* alter */) {} >>+ virtual void alter_def(struct alter_space * /* alter */) noexcept {} >>  /** >>  * Called after creating a new space. Used for performing >>  * long-lasting operations, such as index rebuild or format >>@@ -788,21 +788,21 @@ public: >>  * state to the new space (e.g. move unchanged indexes). >>  * Must not yield or fail. >>  */ >>- virtual void alter(struct alter_space * /* alter */) {} >>+ virtual void alter(struct alter_space * /* alter */) noexcept {} >>  /** >>  * Called after the change has been successfully written >>  * to WAL. Must not fail. >>  */ >>  virtual void commit(struct alter_space * /* alter */, >>- int64_t /* signature */) {} >>+ int64_t /* signature */) noexcept {} >>  /** >>  * Called in case a WAL error occurred. It is supposed to undo >>  * the effect of AlterSpaceOp::prepare and AlterSpaceOp::alter. >>  * Must not fail. >>  */ >>- virtual void rollback(struct alter_space * /* alter */) {} >>+ virtual void rollback(struct alter_space * /* alter */) noexcept {} >>  >>- virtual ~AlterSpaceOp() {} >>+ virtual ~AlterSpaceOp() noexcept {} >>  >>  void *operator new(size_t size) >>  { >>@@ -984,7 +984,7 @@ struct mh_i32_t *AlterSpaceLock::registry; >>  * Replace the old space with a new one in the space cache. >>  */ >> static int >>-alter_space_commit(struct trigger *trigger, void *event) >>+alter_space_commit(struct trigger *trigger, void *event) noexcept >> { >>  struct txn *txn = (struct txn *) event; >>  struct alter_space *alter = (struct alter_space *) trigger->data; >>@@ -1002,12 +1002,8 @@ alter_space_commit(struct trigger *trigger, void *event) >>  * indexes into their new places. >>  */ >>  class AlterSpaceOp *op; >>- try { >>- rlist_foreach_entry(op, &alter->ops, link) >>- op->commit(alter, signature); >>- } catch (Exception *e) { >>- return -1; >>- } >>+ rlist_foreach_entry(op, &alter->ops, link) >>+ op->commit(alter, signature); >I guess this change should be in >«alter: do not catch exceptions when it's not necessary» commit. >>  >>  alter->new_space = NULL; /* for alter_space_delete(). */ >>  /* >>@@ -1029,7 +1025,7 @@ alter_space_commit(struct trigger *trigger, void *event) >>  * alter_space_commit() failure (unlikely) >>  */ >> static int >>-alter_space_rollback(struct trigger *trigger, void * /* event */) >>+alter_space_rollback(struct trigger *trigger, void * /* event */) noexcept >> { >>  struct alter_space *alter = (struct alter_space *) trigger->data; >>  /* Rollback alter ops */ >>@@ -1188,9 +1184,9 @@ alter_space_do(struct txn_stmt *stmt, struct alter_space *alter) >> class CheckSpaceFormat: public AlterSpaceOp >> { >> public: >>- CheckSpaceFormat(struct alter_space *alter) >>+ explicit CheckSpaceFormat(struct alter_space *alter) >>  :AlterSpaceOp(alter) {} >>- virtual void prepare(struct alter_space *alter); >>+ void prepare(struct alter_space *alter) override; >> }; >>  >> void >>@@ -1222,15 +1218,15 @@ public: >>  * names into an old dictionary and deletes new one. >>  */ >>  struct tuple_dictionary *new_dict; >>- virtual void alter_def(struct alter_space *alter); >>- virtual void alter(struct alter_space *alter); >>- virtual void rollback(struct alter_space *alter); >>- virtual ~ModifySpace(); >>+ void alter_def(struct alter_space *alter) noexcept override; >>+ void alter(struct alter_space *alter) noexcept override; >>+ void rollback(struct alter_space *alter) noexcept override; >>+ ~ModifySpace() noexcept override; >> }; >>  >> /** Amend the definition of the new space. */ >> void >>-ModifySpace::alter_def(struct alter_space *alter) >>+ModifySpace::alter_def(struct alter_space *alter) noexcept >> { >>  /* >>  * Use the old dictionary for the new space, because >>@@ -1249,7 +1245,7 @@ ModifySpace::alter_def(struct alter_space *alter) >> } >>  >> void >>-ModifySpace::alter(struct alter_space *alter) >>+ModifySpace::alter(struct alter_space *alter) noexcept >> { >>  /* >>  * Move new names into an old dictionary, which already is >>@@ -1260,12 +1256,12 @@ ModifySpace::alter(struct alter_space *alter) >> } >>  >> void >>-ModifySpace::rollback(struct alter_space *alter) >>+ModifySpace::rollback(struct alter_space *alter) noexcept >> { >>  tuple_dictionary_swap(alter->new_space->def->dict, new_dict); >> } >>  >>-ModifySpace::~ModifySpace() >>+ModifySpace::~ModifySpace() noexcept >> { >>  if (new_dict != NULL) >>  tuple_dictionary_unref(new_dict); >>@@ -1281,9 +1277,9 @@ public: >>  DropIndex(struct alter_space *alter, struct index *index) >>  :AlterSpaceOp(alter), old_index(index) {} >>  struct index *old_index; >>- virtual void alter_def(struct alter_space *alter); >>- virtual void prepare(struct alter_space *alter); >>- virtual void commit(struct alter_space *alter, int64_t lsn); >>+ void alter_def(struct alter_space *alter) noexcept override; >>+ void prepare(struct alter_space *alter) override; >>+ void commit(struct alter_space *alter, int64_t lsn) noexcept override; >> }; >>  >> /* >>@@ -1291,7 +1287,7 @@ public: >>  * the new index from it. >>  */ >> void >>-DropIndex::alter_def(struct alter_space * /* alter */) >>+DropIndex::alter_def(struct alter_space * /* alter */) noexcept >> { >>  rlist_del_entry(old_index->def, link); >> } >>@@ -1305,7 +1301,7 @@ DropIndex::prepare(struct alter_space *alter) >> } >>  >> void >>-DropIndex::commit(struct alter_space *alter, int64_t signature) >>+DropIndex::commit(struct alter_space *alter, int64_t signature) noexcept >> { >>  (void)alter; >>  index_commit_drop(old_index, signature); >>@@ -1323,18 +1319,18 @@ public: >>  :AlterSpaceOp(alter), iid(iid_arg) {} >>  /** id of the index on the move. */ >>  uint32_t iid; >>- virtual void alter(struct alter_space *alter); >>- virtual void rollback(struct alter_space *alter); >>+ void alter(struct alter_space *alter) noexcept override; >>+ void rollback(struct alter_space *alter) noexcept override; >> }; >>  >> void >>-MoveIndex::alter(struct alter_space *alter) >>+MoveIndex::alter(struct alter_space *alter) noexcept >> { >>  space_swap_index(alter->old_space, alter->new_space, iid, iid); >> } >>  >> void >>-MoveIndex::rollback(struct alter_space *alter) >>+MoveIndex::rollback(struct alter_space *alter) noexcept >> { >>  space_swap_index(alter->old_space, alter->new_space, iid, iid); >> } >>@@ -1365,23 +1361,23 @@ public: >>  struct index *old_index; >>  struct index *new_index; >>  struct index_def *new_index_def; >>- virtual void alter_def(struct alter_space *alter); >>- virtual void alter(struct alter_space *alter); >>- virtual void commit(struct alter_space *alter, int64_t lsn); >>- virtual void rollback(struct alter_space *alter); >>- virtual ~ModifyIndex(); >>+ void alter_def(struct alter_space *alter) noexcept override; >>+ void alter(struct alter_space *alter) noexcept override; >>+ void commit(struct alter_space *alter, int64_t lsn) noexcept override; >>+ void rollback(struct alter_space *alter) noexcept override; >>+ ~ModifyIndex() noexcept override; >> }; >>  >> /** Update the definition of the new space */ >> void >>-ModifyIndex::alter_def(struct alter_space *alter) >>+ModifyIndex::alter_def(struct alter_space *alter) noexcept >> { >>  rlist_del_entry(old_index->def, link); >>  index_def_list_add(&alter->key_list, new_index_def); >> } >>  >> void >>-ModifyIndex::alter(struct alter_space *alter) >>+ModifyIndex::alter(struct alter_space *alter) noexcept >> { >>  new_index = space_index(alter->new_space, new_index_def->iid); >>  assert(old_index->def->iid == new_index->def->iid); >>@@ -1397,14 +1393,14 @@ ModifyIndex::alter(struct alter_space *alter) >> } >>  >> void >>-ModifyIndex::commit(struct alter_space *alter, int64_t signature) >>+ModifyIndex::commit(struct alter_space *alter, int64_t signature) noexcept >> { >>  (void)alter; >>  index_commit_modify(new_index, signature); >> } >>  >> void >>-ModifyIndex::rollback(struct alter_space *alter) >>+ModifyIndex::rollback(struct alter_space *alter) noexcept >> { >>  /* >>  * Restore indexes. >>@@ -1416,7 +1412,7 @@ ModifyIndex::rollback(struct alter_space *alter) >>  index_update_def(old_index); >> } >>  >>-ModifyIndex::~ModifyIndex() >>+ModifyIndex::~ModifyIndex() noexcept >> { >>  index_def_delete(new_index_def); >> } >>@@ -1432,15 +1428,15 @@ public: >>  CreateIndex(struct alter_space *alter, struct index_def *def) >>  :AlterSpaceOp(alter), new_index(NULL), new_index_def(def) >>  {} >>- virtual void alter_def(struct alter_space *alter); >>- virtual void prepare(struct alter_space *alter); >>- virtual void commit(struct alter_space *alter, int64_t lsn); >>- virtual ~CreateIndex(); >>+ void alter_def(struct alter_space *alter) noexcept override; >>+ void prepare(struct alter_space *alter) override; >>+ void commit(struct alter_space *alter, int64_t lsn) noexcept override; >>+ ~CreateIndex() noexcept override; >> }; >>  >> /** Add definition of the new key to the new space def. */ >> void >>-CreateIndex::alter_def(struct alter_space *alter) >>+CreateIndex::alter_def(struct alter_space *alter) noexcept >> { >>  index_def_list_add(&alter->key_list, new_index_def); >> } >>@@ -1480,7 +1476,7 @@ CreateIndex::prepare(struct alter_space *alter) >> } >>  >> void >>-CreateIndex::commit(struct alter_space *alter, int64_t signature) >>+CreateIndex::commit(struct alter_space *alter, int64_t signature) noexcept >> { >>  (void) alter; >>  assert(new_index != NULL); >>@@ -1488,7 +1484,7 @@ CreateIndex::commit(struct alter_space *alter, int64_t signature) >>  new_index = NULL; >> } >>  >>-CreateIndex::~CreateIndex() >>+CreateIndex::~CreateIndex() noexcept >> { >>  if (new_index != NULL) >>  index_abort_create(new_index); >>@@ -1521,15 +1517,15 @@ public: >>  struct index_def *new_index_def; >>  /** Old index index_def. */ >>  struct index_def *old_index_def; >>- virtual void alter_def(struct alter_space *alter); >>- virtual void prepare(struct alter_space *alter); >>- virtual void commit(struct alter_space *alter, int64_t signature); >>- virtual ~RebuildIndex(); >>+ void alter_def(struct alter_space *alter) noexcept override; >>+ void prepare(struct alter_space *alter) override; >>+ void commit(struct alter_space *alter, int64_t signature) noexcept override; >>+ ~RebuildIndex() noexcept override; >> }; >>  >> /** Add definition of the new key to the new space def. */ >> void >>-RebuildIndex::alter_def(struct alter_space *alter) >>+RebuildIndex::alter_def(struct alter_space *alter) noexcept >> { >>  rlist_del_entry(old_index_def, link); >>  index_def_list_add(&alter->key_list, new_index_def); >>@@ -1545,7 +1541,7 @@ RebuildIndex::prepare(struct alter_space *alter) >> } >>  >> void >>-RebuildIndex::commit(struct alter_space *alter, int64_t signature) >>+RebuildIndex::commit(struct alter_space *alter, int64_t signature) noexcept >> { >>  struct index *old_index = space_index(alter->old_space, >>  old_index_def->iid); >>@@ -1556,7 +1552,7 @@ RebuildIndex::commit(struct alter_space *alter, int64_t signature) >>  new_index = NULL; >> } >>  >>-RebuildIndex::~RebuildIndex() >>+RebuildIndex::~RebuildIndex() noexcept >> { >>  if (new_index != NULL) >>  index_abort_create(new_index); >>@@ -1600,9 +1596,10 @@ public: >>  TruncateIndex(struct alter_space *alter, uint32_t iid) >>  : AlterSpaceOp(alter), iid(iid), >>  old_index(NULL), new_index(NULL) {} >>- virtual void prepare(struct alter_space *alter); >>- virtual void commit(struct alter_space *alter, int64_t signature); >>- virtual ~TruncateIndex(); >>+ void prepare(struct alter_space *alter) override; >>+ void commit(struct alter_space *alter, >>+ int64_t signature) noexcept override; * Here on a new line. >>+ ~TruncateIndex() noexcept override; >> }; >>  >> void >>@@ -1632,7 +1629,7 @@ TruncateIndex::prepare(struct alter_space *alter) >> } >>  >> void >>-TruncateIndex::commit(struct alter_space *alter, int64_t signature) >>+TruncateIndex::commit(struct alter_space *alter, int64_t signature) noexcept >> { >>  (void)alter; >>  index_commit_drop(old_index, signature); >>@@ -1640,7 +1637,7 @@ TruncateIndex::commit(struct alter_space *alter, int64_t signature) >>  new_index = NULL; >> } >>  >>-TruncateIndex::~TruncateIndex() >>+TruncateIndex::~TruncateIndex() noexcept >> { >>  if (new_index == NULL) >>  return; >>@@ -1657,11 +1654,11 @@ class UpdateSchemaVersion: public AlterSpaceOp >> public: >>  UpdateSchemaVersion(struct alter_space * alter) >>  :AlterSpaceOp(alter) {} >>- virtual void alter(struct alter_space *alter); >>+ void alter(struct alter_space *alter) noexcept override; >> }; >>  >> void >>-UpdateSchemaVersion::alter(struct alter_space *alter) >>+UpdateSchemaVersion::alter(struct alter_space *alter) noexcept >> { >>     (void)alter; >>     ++schema_version; >>@@ -1684,10 +1681,10 @@ public: >>  RebuildCkConstraints(struct alter_space *alter) : AlterSpaceOp(alter), >>  ck_constraint(RLIST_HEAD_INITIALIZER(ck_constraint)) {} >>  struct rlist ck_constraint; >>- virtual void prepare(struct alter_space *alter); >>- virtual void alter(struct alter_space *alter); >>- virtual void rollback(struct alter_space *alter); >>- virtual ~RebuildCkConstraints(); >>+ void prepare(struct alter_space *alter) override; >>+ void alter(struct alter_space *alter) noexcept override; >>+ void rollback(struct alter_space *alter) noexcept override; >>+ ~RebuildCkConstraints() noexcept override; >> }; >>  >> void >>@@ -1716,18 +1713,18 @@ RebuildCkConstraints::space_swap_ck_constraint(struct space *old_space, >> } >>  >> void >>-RebuildCkConstraints::alter(struct alter_space *alter) >>+RebuildCkConstraints::alter(struct alter_space *alter) noexcept >> { >>  space_swap_ck_constraint(alter->old_space, alter->new_space); >> } >>  >> void >>-RebuildCkConstraints::rollback(struct alter_space *alter) >>+RebuildCkConstraints::rollback(struct alter_space *alter) noexcept >> { >>  space_swap_ck_constraint(alter->new_space, alter->old_space); >> } >>  >>-RebuildCkConstraints::~RebuildCkConstraints() >>+RebuildCkConstraints::~RebuildCkConstraints() noexcept >> { >>  struct ck_constraint *old_ck_constraint, *tmp; >>  rlist_foreach_entry_safe(old_ck_constraint, &ck_constraint, link, tmp) { >>@@ -1754,8 +1751,8 @@ class MoveCkConstraints: public AlterSpaceOp >>  struct space *new_space); >> public: >>  MoveCkConstraints(struct alter_space *alter) : AlterSpaceOp(alter) {} >>- virtual void alter(struct alter_space *alter); >>- virtual void rollback(struct alter_space *alter); >>+ void alter(struct alter_space *alter) noexcept override; >>+ void rollback(struct alter_space *alter) noexcept override; >> }; >>  >> void >>@@ -1769,13 +1766,13 @@ MoveCkConstraints::space_swap_ck_constraint(struct space *old_space, >> } >>  >> void >>-MoveCkConstraints::alter(struct alter_space *alter) >>+MoveCkConstraints::alter(struct alter_space *alter) noexcept >> { >>  space_swap_ck_constraint(alter->old_space, alter->new_space); >> } >>  >> void >>-MoveCkConstraints::rollback(struct alter_space *alter) >>+MoveCkConstraints::rollback(struct alter_space *alter) noexcept >> { >>  space_swap_ck_constraint(alter->new_space, alter->old_space); >> } >>@@ -1834,11 +1831,12 @@ public: >>  if (new_id == NULL) >>  diag_raise(); >>  } >>- virtual void prepare(struct alter_space *alter); >>- virtual void alter(struct alter_space *alter); >>- virtual void rollback(struct alter_space *alter); >>- virtual void commit(struct alter_space *alter, int64_t signature); >>- virtual ~CreateConstraintID(); >>+ void prepare(struct alter_space *alter) override; >>+ void alter(struct alter_space *alter) noexcept override; >>+ void rollback(struct alter_space *alter) noexcept override; >>+ void commit(struct alter_space *alter, >>+ int64_t signature) noexcept override; * Here on a new line. >>+ ~CreateConstraintID() noexcept override; >> }; >>  >> void >>@@ -1850,7 +1848,7 @@ CreateConstraintID::prepare(struct alter_space *alter) >> } >>  >> void >>-CreateConstraintID::alter(struct alter_space *alter) >>+CreateConstraintID::alter(struct alter_space *alter) noexcept >> { >>  /* Alter() can't fail, so can't just throw an error. */ >>  if (space_add_constraint_id(alter->old_space, new_id) != 0) >>@@ -1858,14 +1856,15 @@ CreateConstraintID::alter(struct alter_space *alter) >> } >>  >> void >>-CreateConstraintID::rollback(struct alter_space *alter) >>+CreateConstraintID::rollback(struct alter_space *alter) noexcept >> { >>  space_delete_constraint_id(alter->new_space, new_id->name); >>  new_id = NULL; >> } >>  >> void >>-CreateConstraintID::commit(struct alter_space *alter, int64_t signature) >>+CreateConstraintID::commit(struct alter_space *alter, >>+ int64_t signature) noexcept * Here on a new line. >> { >>  (void) alter; >>  (void) signature; >>@@ -1876,7 +1875,7 @@ CreateConstraintID::commit(struct alter_space *alter, int64_t signature) >>  new_id = NULL; >> } >>  >>-CreateConstraintID::~CreateConstraintID() >>+CreateConstraintID::~CreateConstraintID() noexcept >> { >>  if (new_id != NULL) >>  constraint_id_delete(new_id); >>@@ -1891,19 +1890,20 @@ public: >>  DropConstraintID(struct alter_space *alter, const char *name) >>  :AlterSpaceOp(alter), old_id(NULL), name(name) >>  {} >>- virtual void alter(struct alter_space *alter); >>- virtual void commit(struct alter_space *alter , int64_t signature); >>- virtual void rollback(struct alter_space *alter); >>+ void alter(struct alter_space *alter) noexcept override; >>+ void commit(struct alter_space *alter, >>+ int64_t signature) noexcept override; * Here on a new line. >>+ void rollback(struct alter_space *alter) noexcept override; >> }; >>  >> void >>-DropConstraintID::alter(struct alter_space *alter) >>+DropConstraintID::alter(struct alter_space *alter) noexcept >> { >>  old_id = space_pop_constraint_id(alter->old_space, name); >> } >>  >> void >>-DropConstraintID::commit(struct alter_space *alter, int64_t signature) >>+DropConstraintID::commit(struct alter_space *alter, int64_t signature) noexcept >> { >>  (void) alter; >>  (void) signature; >>@@ -1911,7 +1911,7 @@ DropConstraintID::commit(struct alter_space *alter, int64_t signature) >> } >>  >> void >>-DropConstraintID::rollback(struct alter_space *alter) >>+DropConstraintID::rollback(struct alter_space *alter) noexcept >> { >>  if (space_add_constraint_id(alter->new_space, old_id) != 0) { >>  panic("Can't recover after constraint drop rollback (out of " >>-- >>2.7.4 >>  >  >  >-- >Ilya Kosarev >      -- Ilya Kosarev