[tor-commits] [tor/master] prob-distr: Some more comments about the initializers.

[nickm at torproject.org]

commit 27f24484d47cbf72bfad5f81280773128da9e3d2
Author: George Kadianakis <desnacked at riseup.net>
Date:   Mon Mar 25 15:27:03 2019 +0200

    prob-distr: Some more comments about the initializers.
    
    Based on patches and review comments by Riastradh and Catalyst.
    
    Co-authored-by: Taylor R Campbell <campbell+tor at mumble.net>
    Co-authored-by: Taylor Yu <catalyst at torproject.org>
---
 src/lib/math/prob_distr.h | 92 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 87 insertions(+), 5 deletions(-)

diff --git a/src/lib/math/prob_distr.h b/src/lib/math/prob_distr.h
index 9b2ce4124..2eb935e4a 100644
--- a/src/lib/math/prob_distr.h
+++ b/src/lib/math/prob_distr.h
@@ -19,23 +19,100 @@ struct dist {
   const struct dist_ops *ops;
 };
 
-/** Assign the right ops to dist.dist_ops */
+/**
+ * Untyped initializer element for struct dist using the specified
+ * struct dist_ops pointer.  Don't actually use this directly -- use
+ * the type-specific macro built out of DIST_BASE_TYPED below -- but if
+ * you did use this directly, it would be something like:
+ *
+ *   struct weibull mydist = {
+ *     DIST_BASE(&weibull_ops),
+ *     .lambda = ...,
+ *     .k = ...,
+ *   };
+ *
+ * Note there is NO COMPILER FEEDBACK if you accidentally do something
+ * like
+ *
+ *   struct geometric mydist = {
+ *     DIST_BASE(&weibull_ops),
+ *     ...
+ *   };
+ */
 #define DIST_BASE(OPS)  { .ops = (OPS) }
 
-/** A compile-time type-checking macro for use with DIST_BASE_TYPED. */
+/** A compile-time type-checking macro for use with DIST_BASE_TYPED.
+ *
+ *  This macro works by checking that &OBJ is a pointer type that is the same
+ *  type (except for qualifiers) as (const TYPE *)&OBJ. It's a C constraint
+ *  violation (which requires a diagnostic) if two pointers are different types
+ *  and are subtracted. The sizeof() forces compile-time evaluation, and the
+ *  multiplication by zero is to discard the result of the sizeof() from the
+ *  expression.
+ *
+ *  We define this conditionally to suppress false positives from
+ *  Coverity, which gets confused by the sizeof business.
+ */
 #ifdef __COVERITY___
-/* Disable type-checking if coverity is enabled, since they don't like it */
 #define TYPE_CHECK_OBJ(OPS, OBJ, TYPE) 0
 #else
 #define TYPE_CHECK_OBJ(OPS, OBJ, TYPE) \
   (0*sizeof(&(OBJ) - (const TYPE *)&(OBJ)))
 #endif
 
-/** Macro to initialize a distribution with the right OPS, while making sure
- *  that OBJ is of the right TYPE */
+/**
+* Typed initializer element for struct dist using the specified struct
+* dist_ops pointer.  Don't actually use this directly -- use a
+* type-specific macro built out of it -- but if you did use this
+* directly, it would be something like:
+*
+*   struct weibull mydist = {
+*     DIST_BASE_TYPED(&weibull_ops, mydist, struct weibull),
+*     .lambda = ...,
+*     .k = ...,
+*   };
+*
+* If you want to define a distribution type, define a canonical set of
+* operations and define a type-specific initializer element like so:
+*
+*   struct foo {
+*     struct dist base;
+*     int omega;
+*     double tau;
+*     double phi;
+*   };
+*
+*   struct dist_ops foo_ops = ...;
+*
+*   #define FOO(OBJ) DIST_BASE_TYPED(&foo_ops, OBJ, struct foo)
+*
+* Then users can do:
+*
+*   struct foo mydist = {
+*     FOO(mydist),
+*     .omega = ...,
+*     .tau = ...,
+*     .phi = ...,
+*   };
+*
+* If you accidentally write
+*
+*   struct bar mydist = {
+*     FOO(mydist),
+*     ...
+*   };
+*
+* then the compiler will report a type mismatch in the sizeof
+* expression, which otherwise evaporates at runtime.
+*/
 #define DIST_BASE_TYPED(OPS, OBJ, TYPE)                         \
   DIST_BASE((OPS) + TYPE_CHECK_OBJ(OPS,OBJ,TYPE))
 
+/**
+ * Generic operations on distributions.  These simply defer to the
+ * corresponding dist_ops function.  In the parlance of C++, these call
+ * virtual member functions.
+ */
 const char *dist_name(const struct dist *);
 double dist_sample(const struct dist *);
 double dist_cdf(const struct dist *, double x);
@@ -43,6 +120,11 @@ double dist_sf(const struct dist *, double x);
 double dist_icdf(const struct dist *, double p);
 double dist_isf(const struct dist *, double p);
 
+/**
+ * Set of operations on a potentially parametric family of
+ * distributions.  In the parlance of C++, this would be called a
+ * `vtable' and the members are virtual member functions.
+ */
 struct dist_ops {
   const char *name;
   double (*sample)(const struct dist *);