jll63 / yomm2

Fast, orthogonal, open multi-methods. Solve the Expression Problem in C++17.
Boost Software License 1.0
343 stars 18 forks source link

Duplicated definitions lead to dangerous error #36

Closed Rasie1 closed 1 year ago

Rasie1 commented 1 year ago

When you accidentally define method for the same type twice, it compiles and works, but explodes violently when you call the method.

Example:

#include <iostream>
#include <memory>
#include <string>

#include <yorel/yomm2/keywords.hpp>

struct Character {
    virtual ~Character() {
    }
};

struct Warrior : Character { std::string name; };
struct Dog : Character { };

register_classes(Character, Warrior, Dog);

declare_method(std::string, say, (virtual_<const Character&>));

define_method(std::string, say, (const Character& x)) {
    return "hey!";
}

define_method(std::string, say, (const Warrior& x)) {
    return std::string("I'm ") + x.name + "!";
}

define_method(std::string, say, (const Warrior& x)) {
    return "bark!";
}

int main() {
    yorel::yomm2::update_methods();

    {
        auto x = std::make_unique<Warrior>();
        x->name = "john";

        std::cout << say(*x) << std::endl;
    }

    return 0;
}

Output:

a for N5yorel5yomm26methodI11YoMm2_S_sayFNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEENS0_8virtual_IRK9CharacterEEENS0_6policy22hash_factors_in_methodEEE(Segmentation fault (core dumped)

Would be good to come up with some compile-time protection against this

jll63 commented 1 year ago

If you run a debug build, the message is more explicit:

ambiguous call for N5yorel5yomm26methodI11YoMm2_S_sayFNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEENS0_8virtual_IRK9CharacterEEENS0_6policy23hash_factors_in_globalsEEE(7Warrior)
Aborted (core dumped)

Even better when piped into c++filt:

ambiguous call for yorel::yomm2::method<YoMm2_S_say, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > (yorel::yomm2::virtual_<Character const&>), yorel::yomm2::policy::hash_factors_in_globals>(Warrior)

Or, using (undocumented) dev/yomm2filt:

ambiguous call for method<say, basic_string<char, char_traits<char>, allocator<char> > (virtual_<Character const&>)>(Warrior)

Calls to missing or ambiguous definitions (your example is an extreme example of the latter) cannot be detected at compile-time, in general. That is because the definitions may be in different translation units. A language implementation could detect them at link time. YOMM2 detects them during update_methods. If you run a debug build of the program with the (undocumented) YOMM2_TRACE environment variable set to 1, you will get this output:

jll@euclid:~/dev/yomm2$ YOMM2_TRACE=1 /home/jll/dev/yomm2/build/tests/lab 2>&1 | dev/yomm2filt 
Static class info:
  0x55681c6adbd0 Character (Character(0x55681c6adbd0))  
  0x55681c6adc10 Warrior (Character(0x55681c6adbd0), Warrior(0x55681c6adc10))  
  0x55681c6adbe0 Dog (Character(0x55681c6adbd0), Dog(0x55681c6adbe0))  
Inheritance:
  Dog
    bases:      (Character)
    derived:    ()
    covariant_classes: (Dog)
  Warrior
    bases:      (Character)
    derived:    ()
    covariant_classes: (Warrior)
  Character
    bases:      ()
    derived:    (Dog, Warrior)
    covariant_classes: (Warrior, Dog, Character)
Methods:
  method<say, basic_string<char, char_traits<char>, allocator<char> > (virtual_<Character const&>)> (Character(0x55681c6adbd0))
    (const Warrior& long long)
    (const Warrior& long long)
    (const Character& long long)
Layering classes...
  Character
  Dog Warrior
Allocating slots...
  method<say, basic_string<char, char_traits<char>, allocator<char> > (virtual_<Character const&>)>#0: slot 0
    Character Dog Warrior    
Building dispatch table for method<say, basic_string<char, char_traits<char>, allocator<char> > (virtual_<Character const&>)>
  make groups for param #0, class Character
    specs applicable to Warrior
      (const Character& long long)
      (const Warrior& long long)
      (const Warrior& long long)
      -> mask: 111
    specs applicable to Dog
      (const Character& long long)
      -> mask: 001
    specs applicable to Character
      (const Character& long long)
      -> mask: 001
  groups for dim 0:
    group 0/0 mask 001
      Dog(0x55681c6adbe0)
      Character(0x55681c6adbd0)
    group 0/1 mask 111
      Warrior(0x55681c6adc10)
  assigning specs
    group 0/0 mask 001
      Dog(0x55681c6adbe0)
      Character(0x55681c6adbd0)
    select best of:
      (const Character& long long)
    -> (const Character& long long) pf = 0x55681c66dcf0
    group 0/1 mask 111
      Warrior(0x55681c6adc10)
    select best of:
      (const Character& long long)
      (const Warrior& long long)
      (const Warrior& long long)
      ambiguous
  0 not implemented, 1 ambiguities, concrete only: 0, 0
  assigning next
    (const Character& long long):
      for next, select best:
    -> none
    (const Warrior& long long):
      for next, select best:
        (const Character& long long)
    -> (const Character& long long)
    (const Warrior& long long):
      for next, select best:
        (const Character& long long)
    -> (const Character& long long)
Finding hash factor for 3 ti*
trying with M = 2, 4 buckets
found 6814184542283810107 after 1 attempts and 0.018717 msecs
   5 control table
   9 mtbl for Dog: 0x55681c820dc8
  10 mtbl for Warrior: 0x55681c820dd0
  11 mtbl for Character: 0x55681c820e38
Initializing global vector at 0x55681c820de0
   0 pointer to control table
   1 hash table
   5 control table
   9 method<say, basic_string<char, char_traits<char>, allocator<char> > (virtual_<Character const&>)>
   9 mtbl for Dog: 0x55681c820e28
  10 mtbl for Warrior: 0x55681c820e30
  11 mtbl for Character: 0x55681c820e38
  12 end
Optimizing
  method<say, basic_string<char, char_traits<char>, allocator<char> > (virtual_<Character const&>)>
Warrior mtbl[0] = 0x55681c66f8d0 (function)
Dog mtbl[0] = 0x55681c66dcf0 (function)
Character mtbl[0] = 0x55681c66dcf0 (function)
0 not implemented, 1 ambiguities, concrete only: 0, 0
Finished
ambiguous call for method<say, basic_string<char, char_traits<char>, allocator<char> > (virtual_<Character const&>)>(Warrior)

I plan to make this trace feature official soon (but not the exact output format).

Now why doesn't YOMM2 throw an exception, since it knows about the ambiguity? For two reasons: this is consistent with the behavior of C++ in similar situations; and because of dynamic loading. Not so much in your example, but, imagine a pair of ambiguous definitions - for example, add(diagonal_matrix&, matrix&) and add(matrix&, diagonal_matrix&). The program may call update_methods(), then, before trying to add two diagonal matrices, dynamically load a shared object that resolves the ambiguity by adding add(diagonal_matrix&, diagonal_matrix&), and calling update_methods() again.

At some point I plan to expose the data structure corresponding to the trace. The programmer will then be able to throw if problems are detected.