Thread overview
Lexicographical object comparison by selected members of a struct
Aug 21, 2021
Ali Çehreli
Aug 21, 2021
Alexandru Ermicioi
Aug 21, 2021
Alexandru Ermicioi
Aug 21, 2021
Ali Çehreli
Aug 21, 2021
Tejas
Aug 21, 2021
Ali Çehreli
Aug 21, 2021
Tejas
Aug 21, 2021
Ali Çehreli
Aug 21, 2021
Tejas
August 20, 2021
Sometimes I need comparison operators that should consider only some members of a struct:

struct S {
  int year;         // Primary member
  int month;        // Secondary member

  string[] values;  // Irrelevant
}

I've been using the laziest tuple+tupleof solution in some of my structs:

  bool opEquals(const typeof(this) that) {
    import std.typecons : tuple;

    return tuple(this.tupleof).opEquals(tuple(that.tupleof));
  }

  // Similar for opCmp.

That is repetitive, expensive at run time, and does not satisfy a requirement: Some members (like 'values' above) should not be considered during comparison.

I am sure you must have come up with similar solutions like the following code, or perhaps this whole thing exists in Phobos but I just wrote it today... for fun... :) (I think it exists somewhere but I could not find it.)  The code is not used in production yet but it should allow me to do the following:

struct S {
  int year;         // Primary member
  int month;        // Secondary member

  string[] values;  // Irrelevant

  mixin MemberwiseComparison!(year, month);  // 'values' excluded; good
}

What do you think?

I have a feeling that e.g. extracting member names from the strings like "this.foo" with the help of findSplitAfter(members[i].stringof) is pretty suspect. Could I do better?

At least the generated assembly is minimal to my eyes. (Much better than my lazy tuple+tupleof complication! :) )

// This helper function is defined outside of MemberwiseComparison
// because we don't want to mixin such a member function into user
// structs.
private string memberName(string thisName) {
  import std.algorithm : findSplitAfter;
  import std.exception : enforce;
  import std.range : empty;
  import std.format : format;

  const found = thisName.findSplitAfter(".");
  enforce(!found[0].empty,
          format!`Failed to find '.' in "%s"`(thisName));

  return found[1];
}

unittest {
  assert(memberName("this.foo") == "foo");

  // It should throw when no '.' is found.
  import std.exception;
  assertThrown(memberName("woo/hoo"));
}

// Mixes in opEquals() and opCmp() that perform lexicographical
// comparisons according to the order of 'members'.
mixin template MemberwiseComparison(members...) {
  bool opEquals(const typeof(this) that) const {

    // A nested function that makes a comparison expression.
    string makeCode(string name) {
      import std.format : format;

      return format!q{
        const a = this.%s;
        const b = that.%s;

        if (a != b) {
          // Early return at first mismatch
          return false;
        }
      }(name, name);
    }

    // Comparison code per member, which would potentially return an
    // early 'false' result.
    static foreach (i; 0 .. members.length) {{
      mixin (makeCode(memberName(members[i].stringof)));
    }}

    // There was no mismatch if we got here.
    return true;
  }

  int opCmp(const typeof(this) that) const {

    // A nested function that makes a comparison expression.
    string makeCode(string name) {
      import std.format : format;

      return format!q{
        const a = this.%s;
        const b = that.%s;

        if (a < b) {
          // 'this' is before; early return
          return -1;
        }

        if (a > b) {
          // 'this' is after; early return
          return  1;
        }
      }(name, name);
    }

    // Comparison code per member, which would potentially return an
    // early before or after decision.
    static foreach (i; 0 .. members.length) {{
      mixin (makeCode(memberName(members[i].stringof)));
    }}

    // Neither 'this' or 'that' was before if we got here.
    return 0;
  }
}

unittest {
  struct S {
    int i;
    double d;
    string s;

    // Only i and d are considered for this type.
    mixin MemberwiseComparison!(i, d);
  }

  // The order is decided by the first member.
  assert(S(1, 2) < S(2, 2));
  assert(S(3, 2) > S(2, 2));

  // The order is decided by the second member.
  assert(S(1, 2) < S(1, 3));
  assert(S(1, 2) > S(1, 1));

  // Objects are equal regardless of the third member.
  assert(S(7, 42, "hello") == S(7, 42, "world"));
}

void main() {
}

Ali
August 21, 2021
On Saturday, 21 August 2021 at 04:34:46 UTC, Ali Çehreli wrote:
> ...

Consider __traits(identifier, member) instead. This one should return member name as string, removing the need of memberName function.

Also you could have an annotation @Equality.Include for example, and make mixin scan fields for it and include only those fields and methods that have it into compariosn and hashing.

Another problem that may arise is storage in local variable of fields before comparison, it might introduce unnecessary copy.

Regards,
Alexandru.
August 21, 2021
On Saturday, 21 August 2021 at 05:34:59 UTC, Alexandru Ermicioi wrote:
> ...

Also there is no need for mixing string code here. You can get the field using __traits(getMember, this, member).
August 20, 2021
On 8/20/21 10:37 PM, Alexandru Ermicioi wrote:
> On Saturday, 21 August 2021 at 05:34:59 UTC, Alexandru Ermicioi wrote:
>> ...
> 
> Also there is no need for mixing string code here. You can get the field using __traits(getMember, this, member).

Cool! Much better. :)

I could not do

  static foreach (member; members) {{
    // ...
  }}

So I am happy with iterating over 0 .. members.length.

// Mixes in opEquals() and opCmp() that perform lexicographical
// comparisons according to the order of 'members'.
mixin template MemberwiseComparison(members...) {
  bool opEquals(const typeof(this) that) const {

    // Comparison code per member, which would potentially return an
    // early 'false' result.
    static foreach (i; 0 .. members.length) {{
      // mixin (makeCode(__traits(identifier, members[i])));
        enum ident = __traits(identifier, members[i]);

        if (__traits(getMember, this, ident) != __traits(getMember, that, ident)) {
          return false;
        }
    }}

    // There was no mismatch if we got here.
    return true;
  }

  int opCmp(const typeof(this) that) const {

    // Comparison code per member, which would potentially return an
    // early before or after decision.
    static foreach (i; 0 .. members.length) {{
      enum ident = __traits(identifier, members[i]);

      if (__traits(getMember, this, ident) < __traits(getMember, that, ident)) {
        return -1;
      }
      if (__traits(getMember, this, ident) > __traits(getMember, that, ident)) {
        return 1;
      }
    }}

    // Neither 'this' nor 'that' was before if we got here.
    return 0;
  }
}

Ali

August 21, 2021

On Saturday, 21 August 2021 at 06:03:33 UTC, Ali Çehreli wrote:

>

On 8/20/21 10:37 PM, Alexandru Ermicioi wrote:

>

[...]

Cool! Much better. :)

I could not do

[...]

Did you use that double curly bracket in static foreach so that you don't get error for declaring stuff inside static foreach ?

August 20, 2021
On 8/20/21 11:19 PM, Tejas wrote:
> On Saturday, 21 August 2021 at 06:03:33 UTC, Ali Çehreli wrote:
>> On 8/20/21 10:37 PM, Alexandru Ermicioi wrote:
>>> [...]
>>
>> Cool! Much better. :)
>>
>> I could not do
>>
>> [...]
> 
> Did you use that double curly bracket in `static foreach` so that you don't get error for declaring stuff inside `static foreach` ?

Yes. 'static foreach' does not introduce scope, which can be pretty useful. For example, one can define functions at module scope.

The subtle differences between 'static foreach' and '(compile-time) foreach' can be surprising. For example, I don't understand why I can't use

  foreach (i; 0 .. members.length) {
    enum ident = __traits(identifier, members[i]);
    // ...
  }

Error: variable `i` cannot be read at compile time.

I think it should be. :) members.length is compile-time; so, 'i' should be compile time.

And then, why can't 'static foreach' iterate over 'members'?

  static foreach (member; members) {{
    // ...
  }}

Error: value of `this` is not known at compile time

Hm? Well, I am happy that there is always a path through. :)

Ali


August 21, 2021

On Saturday, 21 August 2021 at 06:58:47 UTC, Ali Çehreli wrote:

>

On 8/20/21 11:19 PM, Tejas wrote:

>

[...]

Yes. 'static foreach' does not introduce scope, which can be pretty useful. For example, one can define functions at module scope.

The subtle differences between 'static foreach' and '(compile-time) foreach' can be surprising. For example, I don't understand why I can't use

foreach (i; 0 .. members.length) {
enum ident = __traits(identifier, members[i]);
// ...
}

Error: variable i cannot be read at compile time.

I think it should be. :) members.length is compile-time; so, 'i' should be compile time.

And then, why can't 'static foreach' iterate over 'members'?

static foreach (member; members) {{
// ...
}}

Error: value of this is not known at compile time

Hm? Well, I am happy that there is always a path through. :)

Ali

Maybe it has something to do with that compile-time vs compile-time thing by quickfur on the dlang wiki.

I was more impressed that you found that hack in the first place

August 21, 2021
On 8/21/21 1:31 AM, Tejas wrote:

> I was more impressed that you found that hack in the first place

I can't take credit. :) 'static foreach' had that difference since its inception. The spec says "If a new scope is desired for each expansion, use another set of braces:"

  https://dlang.org/spec/version.html#staticforeach

Ali

August 21, 2021

On Saturday, 21 August 2021 at 13:45:59 UTC, Ali Çehreli wrote:

>

On 8/21/21 1:31 AM, Tejas wrote:

>

I was more impressed that you found that hack in the first
place

I can't take credit. :) 'static foreach' had that difference since its inception. The spec says "If a new scope is desired for each expansion, use another set of braces:"

https://dlang.org/spec/version.html#staticforeach

Ali
I'm an inattentive dunce ;_;