You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Today, we guard the declaration of the dynamic-data-related functions in
comapct_lang_det.h with "#ifdef CLD2_DYNAMIC_MODE":
https://code.google.com/p/cld2/source/browse/trunk/public/compact_lang_det.h
This causes some unfortunate side effects when including CLD2 in another
project: unless building with a single compile pass including all sources, any
separate compilation unit that requires dynamic functionality has to have the
same define when it #includes compact_lang_det.h in order to keep the compiler
happy.
For example, Chromium builds CLD2 separately, then links it into the Chromium
binary; but if CLD2_DYNAMIC_MODE isn't defined in Chromium code that includes
compact_lang_det.h, you get compiler errors like the ones below even if CLD2
itself has been built with the define:
error: 'isDataLoaded' is not a member of 'CLD2'
error: 'loadDataFromRawAddress' is not a member of 'CLD2'
Ideally, the #define guard can be encapsulated entirely within CLD2 so that the
dependent library doesn't need to know about this at all.
The downside is that dependent code might accidentally try to use dynamic mode
even if it isn't available. Throwing exceptions isn't a viable solution, since
some projects disable exceptions when compiling. We'd presumably just have to
define the following behavior if CLD2_DYNAMIC_MODE is not defined:
isDataLoaded: return true
loadDataFromRawAddress: no-op and output a warning to stderr
loadDataFromFile: no-op and output a warning to stderr
This change should be fully backwards compatible, since it doesn't change or
remove any existing function declarations under any circumstances.
Original issue reported on code.google.com by andrewha...@google.com on 23 Jun 2014 at 9:57
The text was updated successfully, but these errors were encountered:
This is looking more important now that Chromium is running into issues that
exacerbate the problem:
https://code.google.com/p/chromium/issues/detail?id=367239
In a complex project that uses CLD2, having to fork the build logic on
CLD2_DYNAMIC_MODE is sub-optimal.
I suggest we go ahead with the fix proposed above, with one caveat: While we're
here, we add a new method:
bool isDataDynamic()
This method can be declared in the .h and implemented with a simple
preprocessor check in the c++ code to return either true (if CLD2_DYNAMIC_MODE)
or false (otherwise). This will still allow dependent projects to be smart. The
cost is that we will need to bring in a total of 5 function signatures into the
header (the 4 that already exist, plus the new one); this shouldn't be a big
deal, though, since there's no extra includes and build optimizations should be
able to simply discard the unused symbols in most projects. Very low cost, and
makes life easier. I'm going to go ahead with this.
Original comment by andrewha...@google.com on 25 Jul 2014 at 8:07
Original issue reported on code.google.com by
andrewha...@google.com
on 23 Jun 2014 at 9:57The text was updated successfully, but these errors were encountered: