Re: [PATCH v4 00/11] Implement and use database "features"

[David Bremner]

Austin Clements  writes:

> This is v4 of id:1406859003-11561-1-git-send-email-amdra...@mit.edu.
> In addition to rebasing to current master, this makes several tidying
> changes: it gives the features enum a name for better
> self-documentation and type-safety; improves the robustness of
> _parse_features to NULL pointers; requests upgrades if the database
> version is old, even if it supports all current features; improves
> various comments; and fixes some style errors.
>

pushed this version of the series to master

d


pgpwnIlJdeASb.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v4 00/11] Implement and use database "features"

[David Bremner]

Austin Clements  writes:

> This is v4 of id:1406859003-11561-1-git-send-email-amdragon at mit.edu.
> In addition to rebasing to current master, this makes several tidying
> changes: it gives the features enum a name for better
> self-documentation and type-safety; improves the robustness of
> _parse_features to NULL pointers; requests upgrades if the database
> version is old, even if it supports all current features; improves
> various comments; and fixes some style errors.
>

pushed this version of the series to master

d
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 647 bytes
Desc: not available
URL: 

[PATCH v4 00/11] Implement and use database "features"

[Austin Clements]

This is v4 of id:1406859003-11561-1-git-send-email-amdragon at mit.edu.
In addition to rebasing to current master, this makes several tidying
changes: it gives the features enum a name for better
self-documentation and type-safety; improves the robustness of
_parse_features to NULL pointers; requests upgrades if the database
version is old, even if it supports all current features; improves
various comments; and fixes some style errors.

The diff from v3 is below.  Most of this diff relates to giving the
enum a name, since it has to move above struct _notmuch_database and
we need to define bitwise operators for C++.

diff --git a/lib/database-private.h b/lib/database-private.h
index 2ffab33..ca0751c 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -36,36 +36,30 @@

 #pragma GCC visibility push(hidden)

-struct _notmuch_database {
-notmuch_bool_t exception_reported;
-
-char *path;
-
-notmuch_database_mode_t mode;
-int atomic_nesting;
-Xapian::Database *xapian_db;
-
-/* Bit mask of features used by this database.  Features are
- * named, independent aspects of the database schema.  This is a
- * bitwise-OR of NOTMUCH_FEATURE_* values (below). */
-unsigned int features;
-
-unsigned int last_doc_id;
-uint64_t last_thread_id;
-
-Xapian::QueryParser *query_parser;
-Xapian::TermGenerator *term_gen;
-Xapian::ValueRangeProcessor *value_range_processor;
-Xapian::ValueRangeProcessor *date_range_processor;
-};
-
-/* Bit masks for _notmuch_database::features. */
-enum {
+/* Bit masks for _notmuch_database::features.  Features are named,
+ * independent aspects of the database schema.
+ *
+ * A database stores the set of features that it "uses" (implicitly
+ * before database version 3 and explicitly as of version 3).
+ *
+ * A given library version will "recognize" a particular set of
+ * features; if a database uses a feature that the library does not
+ * recognize, the library will refuse to open it.  It is assumed the
+ * set of recognized features grows monotonically over time.  A
+ * library version will "implement" some subset of the recognized
+ * features: some operations may require that the database use (or not
+ * use) some feature, while other operations may support both
+ * databases that use and that don't use some feature.
+ *
+ * On disk, the database stores string names for these features (see
+ * the feature_names array).  These enum bit values are never
+ * persisted to disk and may change freely.
+ */
+enum _notmuch_features {
 /* If set, file names are stored in "file-direntry" terms.  If
  * unset, file names are stored in document data.
  *
- * Introduced: version 1.  Implementation support: both for read;
- * required for write. */
+ * Introduced: version 1. */
 NOTMUCH_FEATURE_FILE_TERMS = 1 << 0,

 /* If set, directory timestamps are stored in documents with
@@ -73,7 +67,7 @@ enum {
  * timestamps are stored in documents with XTIMESTAMP terms and
  * absolute paths.
  *
- * Introduced: version 1.  Implementation support: required. */
+ * Introduced: version 1. */
 NOTMUCH_FEATURE_DIRECTORY_DOCS = 1 << 1,

 /* If set, the from, subject, and message-id headers are stored in
@@ -82,7 +76,6 @@ enum {
  * retrieved from the message file.
  *
  * Introduced: optional in version 1, required as of version 3.
- * Implementation support: both.
  */
 NOTMUCH_FEATURE_FROM_SUBJECT_ID_VALUES = 1 << 2,

@@ -90,13 +83,71 @@ enum {
  * unset, folder terms are probabilistic and stemmed and path
  * terms do not exist.
  *
- * Introduced: version 2.  Implementation support: required. */
+ * Introduced: version 2. */
 NOTMUCH_FEATURE_BOOL_FOLDER = 1 << 3,
 };

+/* In C++, a named enum is its own type, so define bitwise operators
+ * on _notmuch_features. */
+inline _notmuch_features
+operator|(_notmuch_features a, _notmuch_features b)
+{
+return static_cast<_notmuch_features>(
+   static_cast(a) | static_cast(b));
+}
+
+inline _notmuch_features
+operator&(_notmuch_features a, _notmuch_features b)
+{
+return static_cast<_notmuch_features>(
+   static_cast(a) & static_cast(b));
+}
+
+inline _notmuch_features
+operator~(_notmuch_features a)
+{
+return static_cast<_notmuch_features>(~static_cast(a));
+}
+
+inline _notmuch_features&
+operator|=(_notmuch_features &a, _notmuch_features b)
+{
+a = a | b;
+return a;
+}
+
+inline _notmuch_features&
+operator&=(_notmuch_features &a, _notmuch_features b)
+{
+a = a & b;
+return a;
+}
+
+struct _notmuch_database {
+notmuch_bool_t exception_reported;
+
+char *path;
+
+notmuch_database_mode_t mode;
+int atomic_nesting;
+Xapian::Database *xapian_db;
+
+/* Bit mask of features used by this database.  This is a
+ * bitwise-OR of NOTMUCH_FEATURE_* values (above). */
+enum _notmuch_features features;
+
+unsigned int last_doc_id;

[PATCH v4 00/11] Implement and use database "features"

[Austin Clements]

This is v4 of id:1406859003-11561-1-git-send-email-amdra...@mit.edu.
In addition to rebasing to current master, this makes several tidying
changes: it gives the features enum a name for better
self-documentation and type-safety; improves the robustness of
_parse_features to NULL pointers; requests upgrades if the database
version is old, even if it supports all current features; improves
various comments; and fixes some style errors.

The diff from v3 is below.  Most of this diff relates to giving the
enum a name, since it has to move above struct _notmuch_database and
we need to define bitwise operators for C++.

diff --git a/lib/database-private.h b/lib/database-private.h
index 2ffab33..ca0751c 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -36,36 +36,30 @@
 
 #pragma GCC visibility push(hidden)
 
-struct _notmuch_database {
-notmuch_bool_t exception_reported;
-
-char *path;
-
-notmuch_database_mode_t mode;
-int atomic_nesting;
-Xapian::Database *xapian_db;
-
-/* Bit mask of features used by this database.  Features are
- * named, independent aspects of the database schema.  This is a
- * bitwise-OR of NOTMUCH_FEATURE_* values (below). */
-unsigned int features;
-
-unsigned int last_doc_id;
-uint64_t last_thread_id;
-
-Xapian::QueryParser *query_parser;
-Xapian::TermGenerator *term_gen;
-Xapian::ValueRangeProcessor *value_range_processor;
-Xapian::ValueRangeProcessor *date_range_processor;
-};
-
-/* Bit masks for _notmuch_database::features. */
-enum {
+/* Bit masks for _notmuch_database::features.  Features are named,
+ * independent aspects of the database schema.
+ *
+ * A database stores the set of features that it "uses" (implicitly
+ * before database version 3 and explicitly as of version 3).
+ *
+ * A given library version will "recognize" a particular set of
+ * features; if a database uses a feature that the library does not
+ * recognize, the library will refuse to open it.  It is assumed the
+ * set of recognized features grows monotonically over time.  A
+ * library version will "implement" some subset of the recognized
+ * features: some operations may require that the database use (or not
+ * use) some feature, while other operations may support both
+ * databases that use and that don't use some feature.
+ *
+ * On disk, the database stores string names for these features (see
+ * the feature_names array).  These enum bit values are never
+ * persisted to disk and may change freely.
+ */
+enum _notmuch_features {
 /* If set, file names are stored in "file-direntry" terms.  If
  * unset, file names are stored in document data.
  *
- * Introduced: version 1.  Implementation support: both for read;
- * required for write. */
+ * Introduced: version 1. */
 NOTMUCH_FEATURE_FILE_TERMS = 1 << 0,
 
 /* If set, directory timestamps are stored in documents with
@@ -73,7 +67,7 @@ enum {
  * timestamps are stored in documents with XTIMESTAMP terms and
  * absolute paths.
  *
- * Introduced: version 1.  Implementation support: required. */
+ * Introduced: version 1. */
 NOTMUCH_FEATURE_DIRECTORY_DOCS = 1 << 1,
 
 /* If set, the from, subject, and message-id headers are stored in
@@ -82,7 +76,6 @@ enum {
  * retrieved from the message file.
  *
  * Introduced: optional in version 1, required as of version 3.
- * Implementation support: both.
  */
 NOTMUCH_FEATURE_FROM_SUBJECT_ID_VALUES = 1 << 2,
 
@@ -90,13 +83,71 @@ enum {
  * unset, folder terms are probabilistic and stemmed and path
  * terms do not exist.
  *
- * Introduced: version 2.  Implementation support: required. */
+ * Introduced: version 2. */
 NOTMUCH_FEATURE_BOOL_FOLDER = 1 << 3,
 };
 
+/* In C++, a named enum is its own type, so define bitwise operators
+ * on _notmuch_features. */
+inline _notmuch_features
+operator|(_notmuch_features a, _notmuch_features b)
+{
+return static_cast<_notmuch_features>(
+   static_cast(a) | static_cast(b));
+}
+
+inline _notmuch_features
+operator&(_notmuch_features a, _notmuch_features b)
+{
+return static_cast<_notmuch_features>(
+   static_cast(a) & static_cast(b));
+}
+
+inline _notmuch_features
+operator~(_notmuch_features a)
+{
+return static_cast<_notmuch_features>(~static_cast(a));
+}
+
+inline _notmuch_features&
+operator|=(_notmuch_features &a, _notmuch_features b)
+{
+a = a | b;
+return a;
+}
+
+inline _notmuch_features&
+operator&=(_notmuch_features &a, _notmuch_features b)
+{
+a = a & b;
+return a;
+}
+
+struct _notmuch_database {
+notmuch_bool_t exception_reported;
+
+char *path;
+
+notmuch_database_mode_t mode;
+int atomic_nesting;
+Xapian::Database *xapian_db;
+
+/* Bit mask of features used by this database.  This is a
+ * bitwise-OR of NOTMUCH_FEATURE_* values (above). */
+enum _notmuch_features features;
+
+unsigned int last_doc_i