[Bug libstdc++/13631] Problems in messages
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631 --- Comment #35 from Jonathan Wakely redi at gcc dot gnu.org --- Author: redi Date: Wed Mar 18 16:17:47 2015 New Revision: 221494 URL: https://gcc.gnu.org/viewcvs?rev=221494root=gccview=rev Log: PR libstdc++/13631 * config/locale/gnu/messages_members.cc (get_glibc_msg): Fix fallback implementation for old glibc. Fix whitespace. Modified: trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/config/locale/gnu/messages_members.cc
[Bug libstdc++/13631] Problems in messages
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631 --- Comment #33 from François Dumont fdumont at gcc dot gnu.org --- Author: fdumont Date: Wed Dec 3 19:47:00 2014 New Revision: 218329 URL: https://gcc.gnu.org/viewcvs?rev=218329root=gccview=rev Log: 2014-12-03 François Dumont fdum...@gcc.gnu.org PR libstdc++/13631 * include/bits/codecvt.h (codecvtchar, char, mbstate_t): friend class std::messageschar. (codecvtwchar_t, char, mbstate_t): friend class std::messageswchar_t. * config/locale/gnu/messages_member.h (messageschar::do_open): Specialized. (messageschar::do_close): Likewise. (messageswchar_t::do_open): Likewise. (messageswchar_t::do_close): Likewise. * config/locale/gnu/messages_member.cc: (messageschar::do_open): Implement. Use bind_textdomain_codeset based on codecvtchar, char, mbstate_t._M_c_locale_codecvt code set. Use internal cache to keep opened domain name with locale information. (messageswchar_t::do_open): Likewise with codecvtwchar_t, char, mbstate_t. (messageschar::do_close): Implement. Clean cache information. (messageswchar_t::do_close): Likewise. (get_glibc_msg): New. Use dgettext rather than gettext using cached domain name associated to catalog id. (messageschar::do_get): Use latter. (messageswchar_t::do_get): Likewise and use also cached locale codecvtwchar_t, char, mbstate_t facet to convert wchar_t default value to char and the result back to wchar_t. * testsuite/22_locale/messages/13631.cc: New. * testsuite/22_locale/messages/members/char/2.cc: Use also fr_FR locale for charset conversion to get the expected accented character. Added: trunk/libstdc++-v3/testsuite/22_locale/messages/13631.cc Modified: trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/config/locale/gnu/messages_members.cc trunk/libstdc++-v3/config/locale/gnu/messages_members.h trunk/libstdc++-v3/include/bits/codecvt.h trunk/libstdc++-v3/testsuite/22_locale/messages/members/char/2.cc
[Bug libstdc++/13631] Problems in messages
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631 François Dumont fdumont at gcc dot gnu.org changed: What|Removed |Added Status|NEW |RESOLVED CC||fdumont at gcc dot gnu.org Resolution|--- |FIXED Target Milestone|--- |5.0 --- Comment #34 from François Dumont fdumont at gcc dot gnu.org --- Now fixed for messageschar and messageswchar_t.
[Bug libstdc++/13631] Problems in messages
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631 --- Comment #31 from Paolo Carlini paolo.carlini at oracle dot com 2011-05-05 21:34:01 UTC --- Created attachment 24194 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=24194 Last patch re-diffed vs current mainline
[Bug libstdc++/13631] Problems in messages
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631 --- Comment #32 from Sam Varshavchik mr...@courier-mta.com 2011-05-05 22:25:50 UTC --- Created attachment 24196 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=24196 Sample test program Here's a simple test program that I threw together. It uses the message catalog from libc and gnupg, both of which should be widely available, and both have extensive localization where anyone can find a good test case in their native tongue, that they can verify. I've used the ru_RU locale for testing purposes, but you can use any locale where the two sample strings have been localized. From the trunk, the third string printed by the test program comes out unlocalized. There's your breakage. The existing breakage is that open() sets the domain globally, the second open() stomps all over the first one, and the third call to get() ends up looking in the wrong domain. After applying Paolo's rebased patch to the trunk, the third string from the test program is now the same string as the first string, which is the expected result. That's all the testing I've done so far, but it looks good.
[Bug libstdc++/13631] Problems in messages
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631 --- Comment #30 from François Dumont fdumont at gcc dot gnu.org 2011-01-14 21:36:34 UTC --- Created attachment 22967 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=22967 locale message facet patch Hi Here is a patch proposition based on revision 168822. I can't apply it because the locale messages facet are not working on my platform, even without any patch. If I change my linux distrib or if they work after the next upgrade I will try to work on it again. For info a regression has been reported using this patch so it is surely not yet ready. François
[Bug libstdc++/13631] Problems in messages
--- Comment #29 from mrsam at courier-mta dot com 2009-06-19 00:47 --- Created an attachment (id=18022) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=18022action=view) Revised revised patch Here's a whack at actually keeping track of different message catalogs. It compiles, but I haven't tested it yet, I haven't yet figured out how to add a new testsuite to the source tree. Testing welcome. -- mrsam at courier-mta dot com changed: What|Removed |Added Attachment #18004|0 |1 is obsolete|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #24 from paolo dot carlini at oracle dot com 2009-06-16 10:07 --- Interesting... Seems a bit too clever to me, but we'll see. I understand this kind of patch would fix uses of std::message as installed in a locale, not, so to speak, stand-alone uses, right? Anyway, could you please also add a testcase for the exact issue you are fixing? (the one available here, frankly I don't like it much, relies on KDE stuff) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #25 from mrsam at courier-mta dot com 2009-06-16 11:07 --- Yes, but, unfortunately, I just realized that this only partially fixes the original issue. This would fix the use case where different parts of the application use different locales, and different instances of std::messages to open different message catalogs. However, the use case of two different message catalogs being opened in the same locale would still be broken, since there would still be one std::messages bound to that locale. So, the application opening a message catalog in the current locale, and a shared library opening a message catalog in the current locale in order to retrieve messages using the shared library's text domain, that's still broken here. The text domain is not really an attribute of the std::messages object, but rather an attribute of the opened catalog. Currently, do_open() always returns 0, and do_get() ignores the catalog parameter. There's the real problem. do_open() needs to return a real handle, and the text domain needs to be associated with the handle (and not saved in the messages object), and do_get() must retrieve the text domain associated with that handle, and pass it to dgettext(). This way, in this particular use case, the application and the shared library will now use different catalog handles, and there's no longer any text domain confusion. It looks to me like there's really no choice here but to wait until libstdc++ major version can be bumped, and the ABI can be changed :-(. This whole thing will have to wait for then. I tried to shoehorn this into the existing ABI, and it simply doesn't fit all the way in. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #26 from peturrun at gmail dot com 2009-06-16 19:51 --- Wouldn't it be easy to implement catalogs using a vector? If do_open adds the catalog name to the vector and returns the index, do_get can get the name back by using the catalog as the index into the vector. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #27 from mrsam at courier-mta dot com 2009-06-16 21:54 --- I thought of that, but using a vector will not be thread safe. Although I don't believe that the C++ standard requires thread safety for std::messages, applications will definitely expect thread safety here. The underlying libintl functions are thread safe. libstdc++ does include some thread and mutex support, but it's not in the current C++ standard, AFAIK, and I'm not sure if mutexes can be used in the localization library. Using a vector does give rise to a couple of other issues, though: * Applications may rely on the libstdc++ existing implementation of ignoring the return value of open(). I don't know if breaking those applications would be permitted. * It's going to be extra-messy. Even though the only change to the class is the addition of a static class member, which should have only minimal ABI considerations, the existing do_open() code is declared in application-exportable headers (but not do_get()). I think that means that existing applications may have the existing do_open() code compiled into them, so this requires the approach in my last patch, of bundling it into the subclass. If someone can confirm that there won't be any issues with using static mutex objects in code that supports std::messages, I can try to write this patch. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #28 from paolo dot carlini at oracle dot com 2009-06-16 22:03 --- mutexes in general can be used and are used in various places in the library (but, for the record, we are currently a bit worried by performance issues having to do with the one for the global locale, we have a PR open about it), just grep, anyway. Thread-local storage could be also put to good use, perhaps. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #16 from mrsam at courier-mta dot com 2009-06-15 11:13 --- After staring at the code for a while, I'm leaning towards thinking that this change does not really change the application ABI, so the soname bump is not needed. As far as I can tell, there are no public members of std::messages, so applications never access instances of std::messages, the only thing they do is invoke the inlined methods, which call the virtual methods. I don't think that the addition of a class member affects the instances' vtable. Applications do not construct instances of std::messages. I don't think anything gets exposed that does that, this is done by libstdc++, so as long as libstdc++ gets rebuilt using the new class definition, that's all that's needed. use_facet() does not expose any code that constructs the class, that's done elsewhere. So, without any class members being accessed, and no constructions or destructions occuring as part of the ABI, scratch the soname bump -- I don't think it's needed. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #17 from paolo dot carlini at oracle dot com 2009-06-15 11:36 --- Maybe it's because I didn't really look carefully at the patch: aren't you adding a new data member to the class? Changing either size or layout of a type specified in the C++ standard definitely changes the ABI, at least in the rather wide sense we are using in this project. You can find more informations here: http://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #18 from mrsam at courier-mta dot com 2009-06-15 21:53 --- Yes, the patch does add a new data member to the class. I see that this would fall under item #8 under prohibited changes, although, as I said, AFAIK it won't actually break binary compatibility with existing applications, for the reasons I outlined. I think I see a way of doing this without changing the existing std::messages class, but it's ugly. Basically -- I think I can subclass std::messages and put the new data member into the subclass (__gnu_internal::messages?), then override all the virtual methods and find all places in libstdc++ that instantiate std::messages, and change them to instantiate __gnu_internal::messages. I was able to find two places in libstdc++ that instantiate std::messages, which would be changed to instantiate __gnu_internal::messages instead. If I missed any, the results won't be catastrophic -- I think anything that falls through the cracks would just fall back to using the existing implementation, and that can always be fixed up later. This patch would be bigger and uglier (and I'd still like my first one better), but before I try to write it up, is there any reason, that I'm missing, why this approach wouldn't work? One other detail -- anyone know which version of glibc first had libintl that implemented dgettext()? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #19 from paolo dot carlini at oracle dot com 2009-06-15 22:06 --- I think we are definitely going to wait for the next ABI, sorry. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #20 from paolo dot carlini at oracle dot com 2009-06-15 22:18 --- One last clarification, maybe necessary because not spelled out (yet) in the docs: really, when we say *ABI* we mean it in a very wide sense, also including linking together objects built with different versions of the headers. All in all, anything changing either the size or the layout of types defined in the C++ standard is certainly a no-no, but there are also many other subtler forbidden changes, as you may easily understand. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #21 from peturrun at gmail dot com 2009-06-15 23:10 --- Isn't it possible to add more data to messages without breaking the ABI by changing the type of one of the existing members? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #22 from paolo dot carlini at oracle dot com 2009-06-15 23:14 --- A patch would help understanding what you exactly mean, at the moment, I'm skeptical. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #23 from mrsam at courier-mta dot com 2009-06-16 03:51 --- Created an attachment (id=18004) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=18004action=view) Revised patch Well, this is approximately what I have in mind. Aside from the formatting style, which I can clean up, as far as I can tell no existing classes get modified. -- mrsam at courier-mta dot com changed: What|Removed |Added Attachment #17994|0 |1 is obsolete|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #11 from mrsam at courier-mta dot com 2009-06-14 14:54 --- The first part of this bug can be solved by using dcgettext(). do_open() needs to save the text domain in the std::messages object, and do_get() needs to use it to invoke dgettext(). The patch appears to be straightforward. I have no idea about the second part of this bug. -- mrsam at courier-mta dot com changed: What|Removed |Added CC||mrsam at courier-mta dot com http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #12 from mrsam at courier-mta dot com 2009-06-14 17:14 --- Created an attachment (id=17994) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17994action=view) Untested patch to fix the first issue Here's an untested patch to fix at least the first issue. I'll try to test it, as soon as I figure out the build system. Includes an soname bump. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #13 from rguenth at gcc dot gnu dot org 2009-06-14 17:56 --- We'll never accept an SONAME bump ;) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #14 from paolo dot carlini at oracle dot com 2009-06-14 18:25 --- That's true, unfortunately, not in the near future, anyway ;) In general, simple patches in this area managing (*) to not break the ABI would be rather quickly accepted, however. (*) When I say managing I mean it: rather dirty tricks are generally allowed, in *.cc files, at least. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
[Bug libstdc++/13631] Problems in messages
--- Comment #15 from mrsam at courier-mta dot com 2009-06-14 18:57 --- Although I'm the last person who'd shy away from dirty tricks, when it suits my purposes, I see none here. The catalog name received by open() needs to be stashed away somewhere, and passed as a parameter to dgettext(), by do_get(). That's the only way to eliminate the global reference, and I don't see any The only possibility I see is to define an entirely new, a replacement facet structure for std::messages, and somehow arrange newly-compiled code to bind to it, then keep both classes around. Existing code would continue to be bound to the old class, and newly-compiled code would then get bound to the new class. I'm not really familiar with the required compiler-fu that would be necessary to pull this off, though. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13631
