Re: [API] Some more cleanup ideas
On 12/06/2012 07:51 PM, Thorsten Behrens wrote:
For large parts of UNO, making one not violate the exception
specification, would look like this:
try {
functions
} catch(...) {
throw uno::RuntimeException(Arrgh! General $FOO error!!1!);
}
That is not what I would call error handling.
But that problem would not be solved just by removing dynamic exception
specifications from C++ code.
Stephan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
Lubos Lunak wrote:
- rtl::OUString
+ OUString
You cannot compare these with exception specifications. The examples above,
barring very corner cases, are only about readability and nothing else, while
exception specifications are not. Arguing that we should remove exception
specifications is more like arguing that we should remove all asserts.
While I'm not standing in the way of keeping them, I still consider
them useless in 99% of all cases (quite in contrast to asserts).
That might be coloured by personal experience, frequency of finding
bugs with it (~zero), and the general unspecificity (or should I
say, thoughtlessness) of their use throughout the API.
For large parts of UNO, making one not violate the exception
specification, would look like this:
try {
functions
} catch(...) {
throw uno::RuntimeException(Arrgh! General $FOO error!!1!);
}
That is not what I would call error handling.
Mixing ivory-tower musing about ES usefulness hand-waving
arguments about developers paying attention to their
self-documenting presence, and the real, actual benefits they bring
(or don't bring) to our UNO API implementation is at least not
getting us the ideal solution. ;)
My 2 cents,
-- Thorsten
pgp6pgOc2V7xV.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
Hi Thorsten, Stephan, Thorsten Behrens píše v Pá 30. 11. 2012 v 17:23 +0100: using namespace com::sun::star would save some 5 additional characters - let me see what a test build yields... ...but would increase potential for ambiguities. Yes, so I went for the css alias in the end. This change has been pushed to master. Please _please_ PLEASE name it api as you proposed first, not css - it nearly killed me when I found out what that means when I started with OOo [that it is not cascade style sheet, but com::sun::star] - think of the newcomers ;-) Thank you, Kendy ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
Jan Holesovsky wrote: Please _please_ PLEASE name it api as you proposed first, not css - it nearly killed me when I found out what that means when I started with OOo [that it is not cascade style sheet, but com::sun::star] - think of the newcomers ;-) Well, that needs some amount of explanation, too, since the nested namespace is still there, and needs to be used when implementing an interface. Personally I like api slightly better, but I simply lack the time today. If someone signs up for that, then let's please also change _all_ occurences of css in implementation code, _and_ the SDK examples. Cheers, -- Thorsten pgpIf5DxXsIWh.pgp Description: PGP signature ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On 12/03/2012 10:32 AM, Jan Holesovsky wrote: using namespace com::sun::star would save some 5 additional characters - let me see what a test build yields... ...but would increase potential for ambiguities. Yes, so I went for the css alias in the end. This change has been pushed to master. Please _please_ PLEASE name it api as you proposed first, not css - it nearly killed me when I found out what that means when I started with OOo [that it is not cascade style sheet, but com::sun::star] - think of the newcomers ;-) Given that the un-abbreviated com.sun.star will still appear in many places (Basic, Java, Python, UNOIDL source; instantiation of old-style services and singletons; implementation of services and singletons; ...), I strongly suggest to use css rather than anything like api. Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On Mon, 2012-12-03 at 10:39 +0100, Thorsten Behrens wrote: Jan Holesovsky wrote: Please _please_ PLEASE name it api as you proposed first, not css - it nearly killed me when I found out what that means when I started with OOo [that it is not cascade style sheet, but com::sun::star] - think of the newcomers ;-) Well, that needs some amount of explanation, too, since the nested namespace is still there, and needs to be used when implementing an interface. Well - IMHO it's far easier to explain 'api' on an ongoing basis than 'com::sun::star::' truncated to 'css' ;-) Personally I like api slightly better, but I simply lack the time today. If someone signs up for that, then let's please also change _all_ occurences of css in implementation code, _and_ the SDK examples. Sounds reasonable to me. ATB, Michael. -- michael.me...@suse.com , Pseudo Engineer, itinerant idiot ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On 03/12/12 10:32, Jan Holesovsky wrote: Hi Thorsten, Stephan, Thorsten Behrens píše v Pá 30. 11. 2012 v 17:23 +0100: using namespace com::sun::star would save some 5 additional characters - let me see what a test build yields... ...but would increase potential for ambiguities. Yes, so I went for the css alias in the end. This change has been pushed to master. Please _please_ PLEASE name it api as you proposed first, not css - it nearly killed me when I found out what that means when I started with OOo [that it is not cascade style sheet, but com::sun::star] - think of the newcomers ;-) actually i think despite that ambiguous expansion css works much better as an abbreviation of com::sun::star than anything else... the vast majority of the API is obviously not related to styles in any way, and you can just look up the definition with ctags or opengrok anyway... ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On 12/03/2012 10:53 AM, Michael Meeks wrote: On Fri, 2012-11-30 at 18:53 +0100, Lubos Lunak wrote: On Friday 30 of November 2012, Stephan Bergmann wrote: I'm not sure this is a good move. ... Which leaves us with the benefit of shorter, less visually cluttered declarations of C++ functions. But, as I argue above, I am not sure that is an overall benefit at all. I'm convinced it is a benefit :-) - and it's interesting; there have been a string of decisions in the past where we have decided to go for more readable code over (IMHO) gross verbosity: - rtl::OUString( RTL_USTRING_VERYLONGNAME( foo ) ) + foo to me is a -massive- improvement in readability. More minor ones are: - rtl::OUString + OUString Many of these have been resisted with the same no benefit argument. I believe that every redundant token we can clarify, and simplify makes reading and getting into the code much easier for new people. Hackers have to spend tons of their time reading and un-tangling code, so maximum readability is key. I agree that the string literal clean-up is a big win, makes it much more pleasant to read and write code, and that I was wrong in considering it syntactic sugar that would likely not be worth the effort. However, in this case, I argue that leaving the dynamic exception specifications in does have a benefit, in a part of the original mail that got elided. In my view the inevitable line-wrapping / 80 char bustage that huge long exception descriptions bring brings a significant impairment of readability. What should read as a simple interface implementation (and to be fair looks reasonably clean and readable as IDL) gets turned into something much fouler in the header impl. The css:: mangling helps in some minor way with this - but I feel it is far better to hide as much as humanly possible of that duplicate guff. IMO, the duplication is beneficial here. Just as we duplicate, say, a method's return type and parameter types from the UNOIDL specification into the C++ implementation -- and not only because we are technically forced to, but also because it makes the resulting C++ code so much more comprehensible. Abbreviating com::sun::star:: to css:: and folding explicitly listed subclasses of RuntimeException can likely go a long way to reduce accidental complexity while keeping the necessary complexity evident. So, in practice, the specifications in our case are like writting out asserts in the code, and the only questions that there should be are whether it makes sense to have such asserts and whether they are worth the code clutter. Quite. Personally - I'm deeply unconvinced that we can afford to have a code-base where all manner of unexpected side-effects remain un-checked at compile-time etc. The only call methods that throw exceptions mentioned above criteria is (to me) far too opaque to have any reasonable chance of reviews and newbie implementers from following it - worse manual checking of that sort of tedious detail, even if it is done is highly error prone: and I suspect is done only one way. Who - when changing a function checks all call-sites of it to see if they declare that exception ? ;-) Where does the only call methods that throw exceptions mentioned above criterion come form? It misses a fundamental part of programming with exceptions, namely the necessity to match exceptions thrown by a function's implementation to the function's interface. Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On 12/02/2012 09:03 PM, Michael Stahl wrote: so... following the above reasoning i have just re-enabled the exception specifications with eb0cfb3bf220892e4885945452930790f5e22000; they are written only in an --enable-dbgutil build. what is still missing then is a macro for use in the API implementations that expands to nothing unless --enable-dbgutil is set; presumably a cleanup to use such a macro everywhere should be done together with replacing the ::com::sun::star in the exception specs with ::css, which should make the clutter a bit less annoying. Sounds like a use-case for a refurbished SAL_WARN, which expands to nothing in exactly those cases where we want to elide runtime checks for unexpected but have no other way to do so (e.g., no -fno-enforce-eh-specs in Clang). And then, it would probably be better to have cppumaker generate SAL_THROW-style exception specifications in all cases, so that e.g. somebody developing C++ code in a GCC --disable-dbgutil scenario would not inadvertently forget those dynamic exception specifications in newly written code. Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On 12/03/2012 04:53 AM, Noel Grandin wrote: Sunday, 2 December 2012, Michael Stahl wrote: what i'd really like to have though is a C++ keyword with semantics of just use the same exception specification as the base class method ... plus error if there is not actually a base class method with that parameter signature, while we're I'm pretty sure recent compilers support an override annotation that does that kind of checking. Note that override addresses the second objective (and we could indeed benefit from a SAL_OVERRIDE; this is similar to how we had macros for back-then new C++ style casts, which over time got replaced again with the true thing) but not the first. Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
Hi all, Michael Meeks píše v Po 03. 12. 2012 v 10:00 +: Well, that needs some amount of explanation, too, since the nested namespace is still there, and needs to be used when implementing an interface. Well - IMHO it's far easier to explain 'api' on an ongoing basis than 'com::sun::star::' truncated to 'css' ;-) I agree :-) Personally I like api slightly better, but I simply lack the time today. If someone signs up for that, then let's please also change _all_ occurences of css in implementation code, _and_ the SDK examples. Sounds reasonable to me. Based on the discussion on the IRC, I've used 'lo::' in the patch, instead of the too generic 'api::'. The patch is split into 2, so that we do not have to patch sc/ immediately due to the ongoing work there. http://artax.karlin.mff.cuni.cz/~kendy/tmp/0001-API-Change-css-to-lo.patch http://artax.karlin.mff.cuni.cz/~kendy/tmp/0002-API-Change-css-to-lo-in-sc-too.patch I have even the 'api::' version around, but it seems to me that generally people seemed to be happier with the less general 'lo::' [and it is shorter ;-)]. Regarding the other concerns - we definitely do not want / cannot change the com.sun.star in the .rdbs etc. due to our goal of keeping as much ABI compatibility as possible. But my view here is that as soon as you start seeing that something is named lo::blah::something in the core, but com::sun::star::blah::something in the rdb or somewhere, you are already advanced enough not to be confused by such a thing. Please let me know if this is acceptable :-) All the best, Kendy ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
Jan Holesovsky wrote: Based on the discussion on the IRC, I've used 'lo::' in the patch, instead of the too generic 'api::'. For the record, I don't like lo. First, it is redundant (LibreOffice, well sure, I knew what I was git-cloning!!1!), second, it is too close to this colloquial English label for the dumping place ... ;) Cheers, -- Thorsten pgpF5kkSn3zVX.pgp Description: PGP signature ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
Hi Thorsten, Thorsten Behrens píše v Po 03. 12. 2012 v 12:53 +0100: Based on the discussion on the IRC, I've used 'lo::' in the patch, instead of the too generic 'api::'. For the record, I don't like lo. First, it is redundant (LibreOffice, well sure, I knew what I was git-cloning!!1!), second, it is too close to this colloquial English label for the dumping place ... ;) OK - so now we are getting to some _real_ bikeshedding ;-) While nobody will be asking what does colloquial English label for the dumping place do in the code, the concern with css is real, mainly around filters. Regarding git cloning - my hope is that in the long run we will be able to transit even the usage of 'com.sun.star' in the Java api and rdb and Python and everywhere some way - and there _some_ prefix fits. I am happy to go ahead with lo, uno, bla, ugh, or whatever that is not widely confused with another technology - ie. no css, com, cpp... Can we end up with something constructive here? Thank you, Kendy ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On Monday 03 of December 2012, Michael Meeks wrote: On Fri, 2012-11-30 at 18:53 +0100, Lubos Lunak wrote: On Friday 30 of November 2012, Stephan Bergmann wrote: I'm not sure this is a good move. ... Which leaves us with the benefit of shorter, less visually cluttered declarations of C++ functions. But, as I argue above, I am not sure that is an overall benefit at all. I'm convinced it is a benefit :-) - and it's interesting; there have been a string of decisions in the past where we have decided to go for more readable code over (IMHO) gross verbosity: - rtl::OUString( RTL_USTRING_VERYLONGNAME( foo ) ) + foo to me is a -massive- improvement in readability. More minor ones are: - rtl::OUString + OUString You cannot compare these with exception specifications. The examples above, barring very corner cases, are only about readability and nothing else, while exception specifications are not. Arguing that we should remove exception specifications is more like arguing that we should remove all asserts. So - IMHO it would be -far- better to have a magic clang plugin to build a database of what methods are called by what other methods, and what signatures they have, and what exceptions they throw as the code compiles - and post build to dump a list of offending methods: I suspect we will have a lot of them to fix. Then leave that on on the tinderbox in perpetuity to enforce cleanliness, allowing us to remove the signatures, make the code more readable, and stop bothering about another set of constraints as we write / review code :-) And of course, I'd love to write that plugin myself if no-one beats me to it ;-) Sounds like an interesting project, mainly because it may turn out to be bloody difficult (consider e.g. calling something via a function pointer). But ok, I'll pass on that one myself :). -- Lubos Lunak l.lu...@suse.cz ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
Sunday, 2 December 2012, Michael Stahl wrote: what i'd really like to have though is a C++ keyword with semantics of just use the same exception specification as the base class method ... plus error if there is not actually a base class method with that parameter signature, while we're I'm pretty sure recent compilers support an override annotation that does that kind of checking. Will find a link later when I'm at work. (Heading off for an early morning cycle right now) ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On 2012-12-02 22:03, Michael Stahl wrote: ... what i'd really like to have though is a C++ keyword plus error if there is not actually a base class method with that parameter signature Here we go, http://stackoverflow.com/questions/497630/safely-override-c-virtual-functions there is a new C++ 11 keyword, override, Which it looks like CLANG supports as of version 3.1, GCC supports it as 4.7, and Visual Studio as of version 11. http://cpprocks.com/a-comparison-of-c11-language-support-in-vs2012-g-4-7-and-clang-3-1/ CLANG even has a tool to automatically add the keyword to existing source code. http://clang-developers.42468.n3.nabble.com/add-override-specifier-example-tool-td4025070.html So perhaps we should define a new macro SAL_OVERRIDE, which expands to the correct keyword on new enough versions of those compilers? Regards, Noel Grandin Disclaimer: http://www.peralex.com/disclaimer.html ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On 11/29/2012 06:44 PM, Thorsten Behrens wrote: Stephan Bergmann wrote: * a bit more extreme - kill ::com::sun::star inside office code, by having a ~global using namespace ::com::sun::star; or namespace api = ::com::sun::star; in e.g. sal/config.h, and writing only _that_ out in the generated c++ headers? (of course keep the old namespace for SDK. And yes, it's no new idea, just a bit less extreme as what's proposed in the wiki) namespace css = com::sun:.star is the de-facto standard there, so yes, I wouldn't mind using that in the generated headers. using namespace com::sun::star would save some 5 additional characters - let me see what a test build yields... ...but would increase potential for ambiguities. Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On 11/29/2012 06:42 PM, Thorsten Behrens wrote: Stephan Bergmann wrote: Right, forgot about the Clang case. So that would mean keeping SAL_THROW non-deprecated, making it a nop with Clang --disable-dbgutil (but making it a non-nop for GCC generally), and changing the cppumaker-generated headers to use SAL_THROW. So the change meanwhile got committed as 0295bd6b3f21dd648af6145ca23d90467f3cec73, and while discussion was ongoing here on irc, I went the bin exception specs entirely route for c++. I concede there's potential debugging utility in having compilers generate runtime checks for exception specs in dbg_util mode, I wonder though if this is worth the mess we'd generate. With SAL_THROW exception specs on api headers removed, there's a very nice substantial cleanup task possible subsequently, that removes it from all implementation methods, too. I'm not sure this is a good move. To be able to programmatically react to an exception raised by a UNO method (which is the raison d'être of non-runtime UNO exceptions), the specification of that method must document the method's behavior with respect to raising that exception, and any implementation of the method must adhere to that specification. However, with that part of a UNO method's interface moved out of sight of a programmer writing a C++ implementation of that method, I fear that adherence to specification will degrade in practice. And that negatively affects an area where we do not shine anyway: reaction to errors. (Which is arguably a tricky area to begin with, but so would probably benefit more from increasing awareness and tooling than from reducing them.) There is indeed a trend in C++ to move away from dynamic exception specifications, but I see none of the problems that motivated that trend affecting us in this specific case. The compiler-induced checks for unexpected that are inherent to dynamic exception specifications and cause space/time overhead can be addressed in production code with -fno-enfore-eh-specs or similar, or could be addressed with SAL_THROW where or similar does not work. There is one concern with the old scheme, namely that exceptions like std::bad_alloc cannot pass out of UNO method implementations, so it is not possible to programmatically react to some isolated operation running out of memory, say. However, for the latter to work much more preparation would be needed (like all the involved functions sporting strong exception guarantees), it is unclear whether those relatively coarse-grained UNO method invocations would not lie outside such programatic catch-and-handle areas anyway, and this could also be made to work with the old scheme, by consistently adding std::exception or similar to the dynamic exception specifications. Which leaves us with the benefit of shorter, less visually cluttered declarations of C++ functions. But, as I argue above, I am not sure that is an overall benefit at all. Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
Stephan Bergmann wrote: using namespace com::sun::star would save some 5 additional characters - let me see what a test build yields... ...but would increase potential for ambiguities. Yes, so I went for the css alias in the end. This change has been pushed to master. Cheers, -- Thorsten pgpj1cpJ4454U.pgp Description: PGP signature ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On Friday 30 of November 2012, Stephan Bergmann wrote: On 11/29/2012 06:42 PM, Thorsten Behrens wrote: With SAL_THROW exception specs on api headers removed, there's a very nice substantial cleanup task possible subsequently, that removes it from all implementation methods, too. I'm not sure this is a good move. To be able to programmatically react to an exception raised by a UNO method (which is the raison d'être of non-runtime UNO exceptions), the specification of that method must document the method's behavior with respect to raising that exception, and any implementation of the method must adhere to that specification. However, with that part of a UNO method's interface moved out of sight of a programmer writing a C++ implementation of that method, I fear that adherence to specification will degrade in practice. And that negatively affects an area where we do not shine anyway: reaction to errors. (Which is arguably a tricky area to begin with, but so would probably benefit more from increasing awareness and tooling than from reducing them.) There is indeed a trend in C++ to move away from dynamic exception specifications, but I see none of the problems that motivated that trend affecting us in this specific case. ... Which leaves us with the benefit of shorter, less visually cluttered declarations of C++ functions. But, as I argue above, I am not sure that is an overall benefit at all. It's been pointed out to me that I'm written as the one to do this change in the LibreOffice4 wiki page. Which was a bit of a surprise to me, given that I don't feel very strongly one way or another (the only thing I felt strongly about a while back was that I didn't want Clang to be the only compiler where the exception specifications actually did anything). Digging in the wiki history, it was Thorsten who added the items related to exception specifications removal, and the link to the C++ article is from Bjoern. I did move the item to the list of things that actually will be done and added my name to it while doing so; my memory is a bit hazy on it, but I think I somehow mistakenly assumed the item was agreed upon and it probably felt like a good thing to try the Clang plugins on. Well, or at least that's the best explanation I can come up with now, I don't remember. That said, from what I can tell, Stephan is correct on the technical details. The problems seen with exception specifications, such as making the app fall flat on its face in case of a problem or being troublesome with templates, are of little or no concern in dbgutil build. In product builds it can be turned off for GCC, MSVC up to at least 2010 doesn't seem to give a damn, and Clang is probably no big deal to handle somehow if seen worth the effort. So, in practice, the specifications in our case are like writting out asserts in the code, and the only questions that there should be are whether it makes sense to have such asserts and whether they are worth the code clutter. -- Lubos Lunak l.lu...@suse.cz ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On 29/11/12 01:54, Thorsten Behrens wrote:
Hi there,
ploughing through offapi cppumaker (the tool that generates the
c++ headers), and so far did this:
* unpublished accessibility API
* renamed XAccessibleEventListener methods
great
* cleansed cppumaker of dead code, RTL_CONSTASCII verbosity, and
writing out exception specs
iirc we want to remove C++ exception specifications for production code
because they don't make sense there - but would it make sense to keep
them in --enable-dbgutil mode? could be useful for debugging... after
all if an exception that isn't documented is thrown that's still a
violation of the API contract.
There remain the following open questions:
* should we keep ~MyClass() {} throw() - or rather have just one
single proper virtual ~XInterface() {} throw in the base class
(note the missing virtual all over the place) - or bin all
exception specs unconditionally?
throw () on a destructor does not hurt imho - it is not allowed to
throw anything in practice...
i'm not aware of any problems by relying on default dtor in derived
classes, but i'm sort of a mere user of C++ so what do i know anyway...
* should we kill [oneway] in IDL while at it? IIRC it went away in
the bridges anyway, via i#116038 or didn't it?
* a bit more extreme - kill ::com::sun::star inside office code, by
having a ~global using namespace ::com::sun::star; or
namespace api = ::com::sun::star; in e.g. sal/config.h, and
writing only _that_ out in the generated c++ headers?
(of course keep the old namespace for SDK. And yes, it's no new
idea, just a bit less extreme as what's proposed in the wiki)
we'd still need the full namespace in the headers bundled with the SDK,
so how would it work to have different one in headers used by LO build?
generate them twice?
why not have some LO header that is included everywhere contain a
namespace css = com::sun::star; and generate headers as before ?
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On 11/29/2012 01:54 AM, Thorsten Behrens wrote:
ploughing through offapi cppumaker (the tool that generates the
c++ headers), and so far did this:
* unpublished accessibility API
* renamed XAccessibleEventListener methods
* cleansed cppumaker of dead code, RTL_CONSTASCII verbosity, and
writing out exception specs
as discussed offline, did you keep the exception specifications as
SAL_THROW comments?
There remain the following open questions:
* should we keep ~MyClass() {} throw() - or rather have just one
single proper virtual ~XInterface() {} throw in the base class
(note the missing virtual all over the place) - or bin all
exception specs unconditionally?
I would refrain from such a massive change as making ~XInterface
virtual. And having nothrow specifications on destructors is fine IMO.
* should we kill [oneway] in IDL while at it? IIRC it went away in
the bridges anyway, via i#116038 or didn't it?
Yes, [oneway] is effectively completely dead by now (compared to almost
dead before the binary URP rewrite).
* a bit more extreme - kill ::com::sun::star inside office code, by
having a ~global using namespace ::com::sun::star; or
namespace api = ::com::sun::star; in e.g. sal/config.h, and
writing only _that_ out in the generated c++ headers?
(of course keep the old namespace for SDK. And yes, it's no new
idea, just a bit less extreme as what's proposed in the wiki)
namespace css = com::sun:.star is the de-facto standard there, so yes, I
wouldn't mind using that in the generated headers.
Stephan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On 11/29/2012 12:37 PM, Michael Stahl wrote:
On 29/11/12 01:54, Thorsten Behrens wrote:
* cleansed cppumaker of dead code, RTL_CONSTASCII verbosity, and
writing out exception specs
iirc we want to remove C++ exception specifications for production code
because they don't make sense there - but would it make sense to keep
them in --enable-dbgutil mode? could be useful for debugging... after
all if an exception that isn't documented is thrown that's still a
violation of the API contract.
Just noted that solenv/gbuild/platform/com_GCC_defs.mk already does
that, setting -fno-enforce-eh-specs unless --enable-dbgutil.
solenv/gbuild/platform/com_MSC_defs.mk strangely uses -EHa (catching SEH
exceptions in addition to C++ exceptions) instead of -EHs (catching just
C++ exceptions) or even -EHsc (in addition, assume C functions to never
throw). I don't know whether MSC has a switch these days similar to
GCC's -fno-enforce-eh-specs (i.e., to avoid emitting code that catches
unexpected exceptions and diverts to std::unexpected) -- IIRC it
traditionally behaved like that per default, but I think that Standard
violation got fixed eventually?
The above does not match well with SAL_THROW as currently defined in
sal/types.h: The latter expands to nothing for GCC and to throw (...)
for MSC. The intention behind that was the same as what has been
discussed above, to avoid the additional unexpected-checks emitted by
the compiler in production code (likely GCC did not have
-fno-enfore-eh-specs back then, Sun CC had to be catered for too, and
MSC was definitely always violating the Standard back then by
effectively ignoring any exception specifications). So I think it makes
sense to deprecate SAL_THROW in favor of plain exception specifications.
(So this obsoletes my other mail asking to keep the exception
specifications as SAL_THROW comments.)
And to keep us honest, it probably makes sense to keep exception
specifications in cppumaker-generated headers after all. The
implementations of those functions need to adhere to the corresponding
UNOIDL method raises-clauses anyway (when interacting with other UNO
environments, or even with old C++ UNO code), and having them checked at
runtime in --enable-dbgutil builds helps identify design bugs in the API
(see e.g. https://bugs.freedesktop.org/show_bug.cgi?id=57611#c6
report builder design mode CRASH on change FixedLine height to zero).
There remain the following open questions:
* should we keep ~MyClass() {} throw() - or rather have just one
single proper virtual ~XInterface() {} throw in the base class
(note the missing virtual all over the place) - or bin all
exception specs unconditionally?
throw () on a destructor does not hurt imho - it is not allowed to
throw anything in practice...
i'm not aware of any problems by relying on default dtor in derived
classes, but i'm sort of a mere user of C++ so what do i know anyway...
The explicitly mentioned dtors in derived classes are there to avoid
warnings about virtual members and non-virtual dtor (made necessary by
the design bug of not having a virtual dtor generated for XInterface).
Stephan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On 29/11/12 14:05, Stephan Bergmann wrote:
On 11/29/2012 12:37 PM, Michael Stahl wrote:
On 29/11/12 01:54, Thorsten Behrens wrote:
* cleansed cppumaker of dead code, RTL_CONSTASCII verbosity, and
writing out exception specs
iirc we want to remove C++ exception specifications for production code
because they don't make sense there - but would it make sense to keep
them in --enable-dbgutil mode? could be useful for debugging... after
all if an exception that isn't documented is thrown that's still a
violation of the API contract.
Just noted that solenv/gbuild/platform/com_GCC_defs.mk already does
that, setting -fno-enforce-eh-specs unless --enable-dbgutil.
The above does not match well with SAL_THROW as currently defined in
sal/types.h: The latter expands to nothing for GCC and to throw (...)
for MSC. The intention behind that was the same as what has been
discussed above, to avoid the additional unexpected-checks emitted by
the compiler in production code (likely GCC did not have
-fno-enfore-eh-specs back then, Sun CC had to be catered for too, and
MSC was definitely always violating the Standard back then by
effectively ignoring any exception specifications). So I think it makes
sense to deprecate SAL_THROW in favor of plain exception specifications.
(So this obsoletes my other mail asking to keep the exception
specifications as SAL_THROW comments.)
also iirc LLVM/clang has no option similar to -fno-enfore-eh-specs, i.e.
it always enforces exception specifications, so if we were to use that
for product builds (no i am not proposing that :) ), we'd need to not
generate the throw in cppumaker or use some macro to nerf it...
There remain the following open questions:
* should we keep ~MyClass() {} throw() - or rather have just one
single proper virtual ~XInterface() {} throw in the base class
(note the missing virtual all over the place) - or bin all
exception specs unconditionally?
throw () on a destructor does not hurt imho - it is not allowed to
throw anything in practice...
i'm not aware of any problems by relying on default dtor in derived
classes, but i'm sort of a mere user of C++ so what do i know anyway...
The explicitly mentioned dtors in derived classes are there to avoid
warnings about virtual members and non-virtual dtor (made necessary by
the design bug of not having a virtual dtor generated for XInterface).
ok i missed that XInterface also has no virtual dtor, and if we add one
now it will break C++ ABI completely, so we're stuck with the status quo
on that...
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
On Thursday 29 of November 2012, Michael Stahl wrote: also iirc LLVM/clang has no option similar to -fno-enfore-eh-specs, i.e. it always enforces exception specifications, so if we were to use that for product builds (no i am not proposing that :) ), we'd need to not generate the throw in cppumaker or use some macro to nerf it... AFAIK Clang is now the official compiler for MacOSX and *BSD, so I guess sooner or later there will be product builds built with it. That said, if it really makes a difference, I haven't look at the source, but I expect implementing -fno-enforce-eh-specs in Clang should be no big deal. -- Lubos Lunak l.lu...@suse.cz ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
Stephan Bergmann wrote: Right, forgot about the Clang case. So that would mean keeping SAL_THROW non-deprecated, making it a nop with Clang --disable-dbgutil (but making it a non-nop for GCC generally), and changing the cppumaker-generated headers to use SAL_THROW. So the change meanwhile got committed as 0295bd6b3f21dd648af6145ca23d90467f3cec73, and while discussion was ongoing here on irc, I went the bin exception specs entirely route for c++. I concede there's potential debugging utility in having compilers generate runtime checks for exception specs in dbg_util mode, I wonder though if this is worth the mess we'd generate. With SAL_THROW exception specs on api headers removed, there's a very nice substantial cleanup task possible subsequently, that removes it from all implementation methods, too. Cheers, -- Thorsten pgpkq3oMnCgY0.pgp Description: PGP signature ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [API] Some more cleanup ideas
Stephan Bergmann wrote: * should we kill [oneway] in IDL while at it? IIRC it went away in the bridges anyway, via i#116038 or didn't it? Yes, [oneway] is effectively completely dead by now (compared to almost dead before the binary URP rewrite). Killing in progress. * a bit more extreme - kill ::com::sun::star inside office code, by having a ~global using namespace ::com::sun::star; or namespace api = ::com::sun::star; in e.g. sal/config.h, and writing only _that_ out in the generated c++ headers? (of course keep the old namespace for SDK. And yes, it's no new idea, just a bit less extreme as what's proposed in the wiki) namespace css = com::sun:.star is the de-facto standard there, so yes, I wouldn't mind using that in the generated headers. using namespace com::sun::star would save some 5 additional characters - let me see what a test build yields... Cheers, -- Thorsten pgpcoJVHEf5fw.pgp Description: PGP signature ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[API] Some more cleanup ideas
Hi there,
ploughing through offapi cppumaker (the tool that generates the
c++ headers), and so far did this:
* unpublished accessibility API
* renamed XAccessibleEventListener methods
* cleansed cppumaker of dead code, RTL_CONSTASCII verbosity, and
writing out exception specs
There remain the following open questions:
* should we keep ~MyClass() {} throw() - or rather have just one
single proper virtual ~XInterface() {} throw in the base class
(note the missing virtual all over the place) - or bin all
exception specs unconditionally?
* should we kill [oneway] in IDL while at it? IIRC it went away in
the bridges anyway, via i#116038 or didn't it?
* a bit more extreme - kill ::com::sun::star inside office code, by
having a ~global using namespace ::com::sun::star; or
namespace api = ::com::sun::star; in e.g. sal/config.h, and
writing only _that_ out in the generated c++ headers?
(of course keep the old namespace for SDK. And yes, it's no new
idea, just a bit less extreme as what's proposed in the wiki)
Thoughts appreciated,
-- Thorsten
pgppYE4jNC7MV.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
