Re: Automatic using ::rtl::OUString etc.
On 07/16/2012 04:03 PM, Lubos Lunak wrote:
On Thursday 12 of April 2012, Lubos Lunak wrote:
would somebody see a problem with this?
sal/inc/rtl/ustring.hxx :
+#ifdef RTL_AUTOMATIC_USING
+using ::rtl::OUString;
+using ::rtl::OStringToOUString;
+using ::rtl::OUStringToOString;
+#endif
I seriously doubt there will ever be any O(U)String anywhere in LO build
that will not be the rtl one, so the need to explicitly qualify it with the
rtl namespace (or go with using, which is what many files do) seem to be
just an unnecessary hassle. So I'd like to do this change for those few
commonly used rtl types which are in practice namespaced by the name itself
(i.e. not the problematic rtl::Reference), and then build all of LO
with -DRTL_AUTOMATIC_USING.
FYI, this is now in. The stable modules (sal/, salhelper/, cppu/,
cppuhelper/) build without it and there's additionally a check including all
their .hxx's to verify them. The rest of LO can now use OUStringfriends
without the explicit rtl.
It turns out that one drawback of this is with header files that are
careful to only declare the incomplete type via
namespace rtl { class OUString; }
(instead of including rtl/ustring.hxx) if that is all they need. They
would either need to continue using rtl::OUString (instead of just
OUString), or need to duplicate the using declaration (which could be
considered breaking of encapsulation), or include rtl/ustring.hxx instead.
Stephan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Automatic using ::rtl::OUString etc.
On Tuesday 18 of September 2012, Stephan Bergmann wrote:
On 07/16/2012 04:03 PM, Lubos Lunak wrote:
FYI, this is now in. The stable modules (sal/, salhelper/, cppu/,
cppuhelper/) build without it and there's additionally a check including
all their .hxx's to verify them. The rest of LO can now use
OUStringfriends without the explicit rtl.
It turns out that one drawback of this is with header files that are
careful to only declare the incomplete type via
namespace rtl { class OUString; }
(instead of including rtl/ustring.hxx) if that is all they need. They
would either need to continue using rtl::OUString (instead of just
OUString), or need to duplicate the using declaration (which could be
considered breaking of encapsulation), or include rtl/ustring.hxx instead.
Is there any practical reason not to include rtl/ustring.hxx? I'd say that
the file eventually ends up included by pretty much all .cxx files, so a
forward declaration of such a basic class does not gain anything.
--
Lubos Lunak
l.lu...@suse.cz
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Automatic using ::rtl::OUString etc.
On 09/18/2012 12:43 PM, Norbert Thiebaud wrote:
On Tue, Sep 18, 2012 at 3:38 AM, Stephan Bergmann sberg...@redhat.com wrote:
It turns out that one drawback of this is with header files that are careful
to only declare the incomplete type via
namespace rtl { class OUString; }
(instead of including rtl/ustring.hxx) if that is all they need. They would
either need to continue using rtl::OUString (instead of just OUString),
or need to duplicate the using declaration (which could be considered
breaking of encapsulation), or include rtl/ustring.hxx instead.
here is (see bottom of the message) a list of cxx file that do
_not_include ustring.hxx (linux build... so there may be a handfull of
platform specific others)
excluding sal and autodoc, that is 236 files... out of 8013 files...
so 97% of the cxx file of the project already include ustring.hxx (and
therefore due to the inclusion chaine also include at a minimum:
* sal/types.h
* sal/config.h
* sal/macros.h
* sal/detail/log.h
* sal/log.hxx
* rtl/ustring.h
* osl/interlck.h
* rtl/string.h
* rtl/textcvt.h
*rtl/textenc.h
* rtl/string.hxx
* osl/diagnose.h
* rtl/memory.h
* rtl/stringutils.hxx
so... out of the 163 header ( git grep -l namespace rtl *{ *class
OU | grep -v OUStringBuffer | wc -l ) that use that namespace rtl {
class OUString; } construct... most of the time they are trying to
avoid a #include that happen anyway.
But the relevant data would rather be how many of those cxx files that
do include rtl/ustring.hxx do so unnecessarily (and are thus needlessly
recompiled when rtl/ustring.hxx changes -- which it happens to do a lot
these days).
Granted, the entities declared in rtl/ustring.hxx are used so widely
that the number of cxx files that do not need to include it is probably
rather small. Still, I consider it good practice in general to use the
idiom of only declaring incomplete types in headers where appropriate.
(And blame not following that practice as one reason for needlessly long
rebuild times.)
Stephan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Automatic using ::rtl::OUString etc.
On Thursday 12 of April 2012, Lubos Lunak wrote: Hello, would somebody see a problem with this? sal/inc/rtl/ustring.hxx : +#ifdef RTL_AUTOMATIC_USING +using ::rtl::OUString; +using ::rtl::OStringToOUString; +using ::rtl::OUStringToOString; +#endif I seriously doubt there will ever be any O(U)String anywhere in LO build that will not be the rtl one, so the need to explicitly qualify it with the rtl namespace (or go with using, which is what many files do) seem to be just an unnecessary hassle. So I'd like to do this change for those few commonly used rtl types which are in practice namespaced by the name itself (i.e. not the problematic rtl::Reference), and then build all of LO with -DRTL_AUTOMATIC_USING. FYI, this is now in. The stable modules (sal/, salhelper/, cppu/, cppuhelper/) build without it and there's additionally a check including all their .hxx's to verify them. The rest of LO can now use OUStringfriends without the explicit rtl. -- Lubos Lunak l.lu...@suse.cz ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Automatic using ::rtl::OUString etc.
On 04/16/2012 11:33 AM, Stephan Bergmann wrote: I am not objecting to the goal of frequently used entities having reasonably short names. I am objecting to the hacky implementation. Just so I don't appear too negative, I really do like your recent work on strings etc., Lubos. Even something like SAL_DEBUG, where I was at first somewhat sceptical and indifferent -- I use it daily now. Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Automatic using ::rtl::OUString etc.
On 04/13/2012 07:42 PM, Lubos Lunak wrote: Strings already kind of are a magic special case. They are the one non-builtin type that is by far the most used one, close to the builtin ones (which is part of the reason why many programming languages do have strings as a builtin type). So I see nothing wrong with trying to get them as conveniently usable as possible, as ::any::little::annoynance::there AddsUpNumerousTimesL BECAUSE_THE_THING_IS_USED_SO_OFTEN. I am not objecting to the goal of frequently used entities having reasonably short names. I am objecting to the hacky implementation. (And, at least in my book, the rtl:: prefix is already reasonably short. What sucks is the O in OUString...) Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Automatic using ::rtl::OUString etc.
On 04/12/2012 03:59 PM, Lubos Lunak wrote: would somebody see a problem with this? sal/inc/rtl/ustring.hxx : +#ifdef RTL_AUTOMATIC_USING +using ::rtl::OUString; +using ::rtl::OStringToOUString; +using ::rtl::OUStringToOString; +#endif I am not too excited about this. For one, we need to ensure that none of the URE published interface implicitly relies on -DRTL_AUTOMATIC_USING. (And it is not clear to me that compiling the sal library with -URTL_AUTOMATIC_USING could even catch all problems in sal headers.) For another, it increases accidental complexity (an ifdef block; yet another -D always passed in) for IMO little gain. And for a third, it introduces a magic special case (certain names from the rtl namespace can be used without qualification; without this being evident from the C++ source code itself, as it relies on .mk file behavior; but there is an exception, that this special case must not be used in certain headers). Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Automatic using ::rtl::OUString etc.
On Fri, Apr 13, 2012 at 08:21:35AM +0200, Stephan Bergmann wrote: On 04/12/2012 03:59 PM, Lubos Lunak wrote: would somebody see a problem with this? sal/inc/rtl/ustring.hxx : +#ifdef RTL_AUTOMATIC_USING +using ::rtl::OUString; +using ::rtl::OStringToOUString; +using ::rtl::OUStringToOString; +#endif I am not too excited about this. For one, we need to ensure that none of the URE published interface implicitly relies on -DRTL_AUTOMATIC_USING. (And it is not clear to me that compiling the sal library with -URTL_AUTOMATIC_USING could even catch all problems in sal headers.) For another, it increases accidental complexity (an ifdef block; yet another -D always passed in) for IMO little gain. And for a third, it introduces a magic special case (certain names from the rtl namespace can be used without qualification; without this being evident from the C++ source code itself, as it relies on .mk file behavior; but there is an exception, that this special case must not be used in certain headers). I tend to agree on all points. If something like that is essential, we could have something like a: salhelper/inc/rtl/stringhelper.hxx: #include rtl/ustring.hxx using ::rtl::OUString; using ::rtl::OStringToOUString; using ::rtl::OUStringToOString; and allow that to be used in .cxx only but not in .hxx. Then again the gain is limited by that and its not automatically checkable without some ugly extra scripting (right?). Best, Bjoern ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Automatic using ::rtl::OUString etc.
On the other hand, any kind of duplication is always bad, isn't it, as is inconsistency. So isn't it, from that point of view, bad that some percentage of the source files contain those using statements (duplication), some a subset of them (inconsistency), others not, some use ::rtl::foo, others rtl::foo, etc etc. Any changes that would make the coding style more consistent and less verbose would be good in my opinion. --tml ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Automatic using ::rtl::OUString etc.
On 04/13/2012 10:13 AM, Bjoern Michaelsen wrote: I tend to agree on all points. If something like that is essential, we could have something like a: salhelper/inc/rtl/stringhelper.hxx: #includertl/ustring.hxx using ::rtl::OUString; using ::rtl::OStringToOUString; using ::rtl::OUStringToOString; and allow that to be used in .cxx only but not in .hxx. Note that it would not hurt too much (at least concerning the published URE API) if that header happened to be included from other headers. Somewhat similar to the standard header approach, where legacy foo.h wraps foo and injects its names into global namespace. Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Automatic using ::rtl::OUString etc.
On 04/13/2012 10:24 AM, Tor Lillqvist wrote: On the other hand, any kind of duplication is always bad, isn't it, as is inconsistency. So isn't it, from that point of view, bad that some percentage of the source files contain those using statements (duplication), some a subset of them (inconsistency), others not, some use ::rtl::foo, others rtl::foo, etc etc. Any changes that would make the coding style more consistent and less verbose would be good in my opinion. Inconsistency in this area is a direct consequence of C++'s somewhat TIMTOWTDI approach how to write names for things. More consistent usage of using directives across the code base would definitely not hurt. But if you wanted to enforce consistency, you would need to abandon namespaces. (And note that Lubos' proposal would not automatically eliminate inconsistency, either, as writing rtl::OUString would technically still be allowed.) Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Automatic using ::rtl::OUString etc.
On Friday 13 of April 2012, Stephan Bergmann wrote: On 04/12/2012 03:59 PM, Lubos Lunak wrote: would somebody see a problem with this? sal/inc/rtl/ustring.hxx : +#ifdef RTL_AUTOMATIC_USING +using ::rtl::OUString; +using ::rtl::OStringToOUString; +using ::rtl::OUStringToOString; +#endif I am not too excited about this. For one, we need to ensure that none of the URE published interface implicitly relies on -DRTL_AUTOMATIC_USING. (And it is not clear to me that compiling the sal library with -URTL_AUTOMATIC_USING could even catch all problems in sal headers.) I think you are right about -URTL_AUTOMATIC_USING for sal not being a very good idea, it'd be better to just have one check .cxx that'd include everything in sal (and another one for URE, and whenever else needed), without having the #define in effect. That'd make sure no problem slips through if the file was automatically made to include inc/*.hxx , and a failure only in the one .cxx would make it more obvious why it fails. For another, it increases accidental complexity (an ifdef block; yet another -D always passed in) for IMO little gain. No, you see it backwards :). It reduces code annoyances for IMO very little price. And for a third, it introduces a magic special case (certain names from the rtl namespace can be used without qualification; Strings already kind of are a magic special case. They are the one non-builtin type that is by far the most used one, close to the builtin ones (which is part of the reason why many programming languages do have strings as a builtin type). So I see nothing wrong with trying to get them as conveniently usable as possible, as ::any::little::annoynance::there AddsUpNumerousTimesL BECAUSE_THE_THING_IS_USED_SO_OFTEN. I understand that this argument probably doesn't work that well with people who have already gotten used to all kind of quirks of a codebase that has managed to get even int and bool types complicated, since we simply have to get the work done somehow (and I said 'we', because I can already watch myself getting used to things I'd prefer not to), but that doesn't mean everybody has to suck it up until the end of time. As the strings are already namespaced by the name itself, I'd be open to alternate solutions such as moving them out of the namespace, but that'd break binary compatibility and the (probably only hypothetical) case of some other code using the name for something else. -- Lubos Lunak l.lu...@suse.cz ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Automatic using ::rtl::OUString etc.
On Friday 13 of April 2012, Bjoern Michaelsen wrote: I tend to agree on all points. If something like that is essential, we could have something like a: salhelper/inc/rtl/stringhelper.hxx: #include rtl/ustring.hxx using ::rtl::OUString; using ::rtl::OStringToOUString; using ::rtl::OUStringToOString; and allow that to be used in .cxx only but not in .hxx. Then again the gain is limited by that and its not automatically checkable without some ugly extra scripting (right?). How is this in practice different from my proposal, besides the two limitations being bigger? -- Lubos Lunak l.lu...@suse.cz ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Automatic using ::rtl::OUString etc.
Hello, would somebody see a problem with this? sal/inc/rtl/ustring.hxx : +#ifdef RTL_AUTOMATIC_USING +using ::rtl::OUString; +using ::rtl::OStringToOUString; +using ::rtl::OUStringToOString; +#endif I seriously doubt there will ever be any O(U)String anywhere in LO build that will not be the rtl one, so the need to explicitly qualify it with the rtl namespace (or go with using, which is what many files do) seem to be just an unnecessary hassle. So I'd like to do this change for those few commonly used rtl types which are in practice namespaced by the name itself (i.e. not the problematic rtl::Reference), and then build all of LO with -DRTL_AUTOMATIC_USING. -- Lubos Lunak l.lu...@suse.cz diff --git a/sal/Library_sal.mk b/sal/Library_sal.mk index 0b6e30f..eaacf07 100644 --- a/sal/Library_sal.mk +++ b/sal/Library_sal.mk @@ -54,6 +54,11 @@ $(eval $(call gb_Library_add_defs,sal,\ -DSAL_DLLIMPLEMENTATION \ )) +# ensure that public headers build without this feature +$(eval $(call gb_Library_add_defs,sal,\ + -URTL_AUTOMATIC_USING \ +)) + $(eval $(call gb_Library_use_libraries,sal,\ $(if $(filter $(GUI),UNX), \ $(if $(filter $(OS),ANDROID),, \ diff --git a/sal/inc/rtl/strbuf.hxx b/sal/inc/rtl/strbuf.hxx index 7711308..ec48088 100644 --- a/sal/inc/rtl/strbuf.hxx +++ b/sal/inc/rtl/strbuf.hxx @@ -849,6 +849,10 @@ typedef rtlunittest::OStringBuffer OStringBuffer; #undef RTL_STRING_CONST_FUNCTION #endif +#ifdef RTL_AUTOMATIC_USING +using ::rtl::OStringBuffer; +#endif + #endif /* __cplusplus */ #endif /* _RTL_STRBUF_HXX_ */ diff --git a/sal/inc/rtl/string.hxx b/sal/inc/rtl/string.hxx index 9791a8c..84d9eba 100644 --- a/sal/inc/rtl/string.hxx +++ b/sal/inc/rtl/string.hxx @@ -1467,6 +1467,11 @@ struct OStringHash } /* Namespace */ +#ifdef RTL_AUTOMATIC_USING +using ::rtl::OString; +using ::rtl::OStringHash; +#endif + #endif /* _RTL_STRING_HXX_ */ /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sal/inc/rtl/ustrbuf.hxx b/sal/inc/rtl/ustrbuf.hxx index a887107..cbfcb5c 100644 --- a/sal/inc/rtl/ustrbuf.hxx +++ b/sal/inc/rtl/ustrbuf.hxx @@ -962,6 +962,10 @@ typedef rtlunittest::OUStringBuffer OUStringBuffer; } #endif +#ifdef RTL_AUTOMATIC_USING +using ::rtl::OUStringBuffer; +#endif + #endif /* _RTL_USTRBUF_HXX_ */ /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sal/inc/rtl/ustring.hxx b/sal/inc/rtl/ustring.hxx index 883fb93..1c9d34a 100644 --- a/sal/inc/rtl/ustring.hxx +++ b/sal/inc/rtl/ustring.hxx @@ -2083,6 +2083,13 @@ inline OString OUStringToOString( const OUString rUnicode, } /* Namespace */ +#ifdef RTL_AUTOMATIC_USING +using ::rtl::OUString; +using ::rtl::OUStringHash; +using ::rtl::OStringToOUString; +using ::rtl::OUStringToOString; +#endif + #endif /* _RTL_USTRING_HXX */ // Include the ostream operator directly here, so that it's always available diff --git a/solenv/gbuild/platform/WNT_INTEL_MSC.mk b/solenv/gbuild/platform/WNT_INTEL_MSC.mk index 0df48c4..1f492fb 100644 --- a/solenv/gbuild/platform/WNT_INTEL_MSC.mk +++ b/solenv/gbuild/platform/WNT_INTEL_MSC.mk @@ -59,6 +59,7 @@ gb_COMPILERDEFS := \ -DBOOST_MEM_FN_ENABLE_CDECL \ -DCPPU_ENV=msci \ -DM1500 \ + -DRTL_AUTOMATIC_USING \ gb_CPUDEFS := -D_X86_=1 diff --git a/solenv/gbuild/platform/com_GCC_defs.mk b/solenv/gbuild/platform/com_GCC_defs.mk index 36b2cc7..435030a 100644 --- a/solenv/gbuild/platform/com_GCC_defs.mk +++ b/solenv/gbuild/platform/com_GCC_defs.mk @@ -85,6 +85,7 @@ gb_COMPILERDEFS := \ -D$(COM) \ -DCPPU_ENV=gcc3 \ -DGXX_INCLUDE_PATH=$(GXX_INCLUDE_PATH) \ + -DRTL_AUTOMATIC_USING \ gb_CFLAGS_COMMON := \ -Wall \ ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
