s-yata / marisa-trie

MARISA: Matching Algorithm with Recursively Implemented StorAge
Other
506 stars 89 forks source link

Inline Agent::set_query to prepare for #27 #31

Closed jmr closed 4 years ago

s-yata commented 4 years ago

marisa/grimoire/trie/state.h is not a public header. It must not be included from a public header (marisa/agent.h).

jmr commented 4 years ago

I thought in https://github.com/s-yata/marisa-trie/pull/27#issuecomment-645737535 you were asking for Agent::set_query(const char *) to be made inline.

If I replace the #include by the forward declaration, then I get this compilation error:

In file included from agent.cc:3:
../../include/marisa/agent.h: In member function ‘void marisa::Agent::set_query(const char*)’:
../../include/marisa/agent.h:32:13: error: invalid use of incomplete type ‘class marisa::grimoire::trie::State’
   32 |       state_->reset();
      |             ^~
../../include/marisa/agent.h:12:7: note: forward declaration of ‘class marisa::grimoire::trie::State’
   12 | class State;
      |       ^~~~~

This kind of thing happens a lot and people just put stuff in an internal or detail namespace to indicate it shouldn't be relied on. That's what grimoire seems to be, but I guess it's less clear that shouldn't be used. Maybe you can think of a better solution.

s-yata commented 4 years ago

I'm sorry that I disturbed you due to my late response.

I was just worried about ambiguity due to overloading, because I was not familiar with overload resolution (https://en.cppreference.com/w/cpp/language/overload_resolution). Now I think that the new gettters may not cause ambiguity problems.