> On March 3, 2016, 4:51 p.m., Aleix Pol Gonzalez wrote: > > src/kcatalog.cpp, line 150 > > <https://git.reviewboard.kde.org/r/127265/diff/1/?file=447929#file447929line150> > > > > Wouldn't it be the same to define it like this? > > ``` > > #if defined(__USE_GNU_GETTEXT) > > extern "C" int Q_DECL_IMPORT _nl_msg_cat_cntr; > > #endif > > ``` > > > > This way we wouldn't have to make special cases for each platform. > > > > I just tried and works here (on Linux+Clang). > > Andre Heinecke wrote: > Ah nice, did not know / remembered this macro. I think in that case we > can also drop the extern int declaration inside the function as we already > declare it now in a platform independent manner as an external variable. > > I've tested this again with mingw and GNU/Linux GCC and it still compiled. > > Andreas Cord-Landwehr wrote: > Just tested, also works fine under Android.
+1 I think the bug in the last review change was that the extern declaration was inside the function. I'm not sure if the #ifdef was a real problem as you probably need the __declspec() also with MinGW. This solution is however the best so far :) - Kåre ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127265/#review93104 ----------------------------------------------------------- On March 3, 2016, 5:49 p.m., Andre Heinecke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127265/ > ----------------------------------------------------------- > > (Updated March 3, 2016, 5:49 p.m.) > > > Review request for KDE Frameworks, Andreas Cord-Landwehr and Kåre Särs. > > > Repository: ki18n > > > Description > ------- > > In review Request 127048 (Restrict _nl_msg_cat_cntr use to GNU gettext > implentations.) Unrelated Windows changes were requested and then introduced > which in my opinion were wrong (and in the opinion of my compiler ;-) ). > > You can't just do an extern "C" dllimport declaration inside a function body: > > src/kcatalog.cpp:190:16: error: expected unqualified-id before string constant > extern "C" int __declspec(dllimport) _nl_msg_cat_cntr; > > > Also that patch changed the Logic from an MSVC specific define to general > Q_OS_WIN. This appears to be wrong, too. While I'm only testing with MinGW > I'm pretty sure that the ifndef _MSC_VER was there for a reason. > > This patch reverts the declaration move and logic change while keeping the > __USE_GNU_GETTEXT guard. > > > Diffs > ----- > > src/kcatalog.cpp 083443d > > Diff: https://git.reviewboard.kde.org/r/127265/diff/ > > > Testing > ------- > > Compiled with mingw and __USE_GNU_GETTEXT for Windows. Also compiled for > GNU/Linux. > > > Thanks, > > Andre Heinecke > >
_______________________________________________ Kde-frameworks-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
