rrthomas / enchant

enchant spellchecking library
http://rrthomas.github.io/enchant/
GNU Lesser General Public License v2.1
347 stars 60 forks source link

Warning using libenchant in other software since 2.7 #375

Closed ctrlaltca closed 7 months ago

ctrlaltca commented 7 months ago

Hi, I'm a developer of KVIrc and we are using libenchant to provide spellchecker support (relevant code)

CI builds started to fail after the update to libenchant 2.7, due to the enchant-provider.h header not being provided/installed anymore. Removing this include the build succeed, but with the following warnings:

KVIrc/src/modules/../kvilib/core/KviPointerList.h:1018:25: warning: possible problem detected in invocation of ‘operator delete’ [-Wdelete-incomplete]
 1018 |                         delete pAuxData;
      |                         ^~~~~~~~~~~~~~~
KVIrc/src/modules/../kvilib/core/KviPointerList.h:1000:27: warning: ‘pAuxData’ has incomplete type
 1000 |                 const T * pAuxData;
      |                           ^~~~~~~~
In file included from KVIrc/src/modules/spellchecker/libkvispellchecker.cpp:28:
/usr/include/enchant-2/enchant.h:42:16: note: forward declaration of ‘struct _EnchantDict’
   42 | typedef struct _EnchantDict   EnchantDict;
      |                ^~~~~~~~~~~~
KVIrc/src/modules/../kvilib/core/KviPointerList.h:1018:25: note: neither the destructor nor the class-specific ‘operator delete’ will be called, even if they are declared when the class is defined
 1018 |                         delete pAuxData;
      |                         ^~~~~~~~~~~~~~~

The proper definition of struct _EnchantDict is in enchant-provider.h. While this is only a warning and is caused by the c++ compiler trying to be too smart, i thought you may want to know of this warning. I'm already working on switching the code to using enchant++.h, that implements a class with a proper destructor.

rrthomas commented 7 months ago

Thanks for raising this issue! I'm a bit puzzled, since application code should not be allocating or deallocating EnchantDicts. That is why the definition is in enchant-provider.h: only providers need it. The provider API has changed, and I've made it private, hence enchant-provider.h is no longer installed.

My C++ knowledge is quite basic, but you appear to have a list of EnchantDict, which surprises me, although it's a KviPointerList, so maybe it's really a list of EnchantDict *?

ctrlaltca commented 7 months ago

Yes, it's a list of EnchantDict *. Our problem is due to the fact that KviPointerList has a method to delete the pointers, that would fail since it doesn't know the exact definition of the struct. This won't work anyway, since enchant_broker_free_dict() should be used instead. Anyway, migrating to enchant++.h has been straightforward: https://github.com/kvirc/KVIrc/pull/2636/files Thank you

rrthomas commented 7 months ago

OK, so it sounds like that wasn't the API's fault, it was a misuse.

Glad you find the C++ API useful (as far as I can tell, most non-GNOME projects that use Enchant use it).