[dev] Warning-free code status
Hi all, http://wiki.services.openoffice.org/wiki/Warning-free_code_status tracks the status of that project. If you own one of the tasks listed there, or are working to make some other platform warning-free, you are invited to add any relevant data and help keep the table up to date. Thanks, -Stephan - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
[dev] Warning-free code CWS naming convention request
Hi folks, if possible please name your CWS which are related to to the warning-free code project in a recognizable manner, e.g Warnings0x. This helps me - and others in the QA - to do a quicker estimate on the effort required to check the CWS and simplifies planning. Thanks Joerg -- *** Sun Microsystems GmbH Joerg W. Skottke Nagelsweg 55 Quality Assurance Engineer 20097 Hamburg Phone: (+49 40) 23646 631 GermanyFax: (+49 40) 23646 550 http://www.sun.de/staroffice mailto:[EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
[dev] Is override via dominance useful (was: [dev] Warning free code: some more warnings that might be disabled)
Frank Schönheit - Sun Microsystems Germany [EMAIL PROTECTED] writes:
(What I intended to say was: what I've learned during reading zillions
of line of code inherited from others is that patterns which are
obscured from plain sight are hard to maintain, and thus prone to
introducing errors. While using cool language features is, well, cool,
its usefulness sometimes decreases over time, when it comes to
maintaining the code. KISS. But hey, I'm digressing.)
Generally, true. But aren't we c++ programmers accustomed to an
unprecedented level of obscureness, anyway (at least compared to, say,
a Java coder)? ;-)
So, in this case, about the only change that will break things with
this idiom (and doesn't trigger a compiler error) is that someone
implements method a() or b() in Interface:
struct Interface
{
- virtual void a() = 0;
+ virtual void a() {}
- virtual void b() = 0;
+ virtual void b() {}
};
struct DerivedInterface : public Interface
{
virtual void c() = 0;
};
struct InterfaceHelper : public virtual Interface
{
virtual void a();
virtual void b();
};
struct DerivedInterfaceImplementation :
public virtual DerivedInterface,
protected InterfaceHelper
{
virtual c();
};
and expects that those are called in DerivedInterfaceImplementation,
instead of the overrides from InterfaceHelper.
Admittedly, this language feature _is_ dangerous when using multiple,
virtual inheritance with concrete classes (instead of interfaces, as
in my idiom), because then, figuring out by hand which method wins can
get quite involved. But I don't think we do that now, and we certainly
shouldn't do that for new code (since there are far more pitfalls with
that than this one).
Cheers,
--
Thorsten
If you're not failing some of the time, you're not trying hard enough.
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: some more warnings that might be disabled
Thorsten Behrens wrote: Hi folks, while trying to get basebmp and vigra warning free, I've been bitten by the following two msvc warnings: - Compiler Warning (level 3) C4686 'user-defined type' : possible change in behavior, change in UDT return calling convention A class template specialization was not is defined before it was used in a return type. Anything that instantiates the class will resolve C4686; declaring an instance or accessing a member (Cint::anything) are also options. - Compiler Warning (level 4) C4668 Error Message 'symbol' is not defined as a preprocessor macro, replacing with '0' for 'directives' A symbol that was not defined was used with a preprocessor directive. The symbol will evaluate to false. To define a symbol, you can use either the #define directive or /D compiler option. - C4686 is one of those infamous 'let's warn our users, because we've changed behaviour and fixed a nonconformant behaviour' MS noise generators (it's about which calling convention is used for the not-yet-instantiated return type). I'd say could safely be disabled. Agreed that such warnings should be disabled globally. (I also want to disable warning C4347: behavior change: 'overload A' is called instead of 'overload B' on CWS sb59, which is for example triggered by some instances of std::minsal_Int32(a,b).) C4668 can potentially unearth real portability problems (that a try to access an undefined preprocessor macro evaluates to '0' is a convention possibly not shared by all c++ compilers of this universe) This behavior is not a convention, but mandated by the C/C++ standards. Still, in all the cases I encountered so far where MSC issued this warning, it either (a) unearthed really broken code like #if STRCODE==sal_Unicode in tools/source/string/strimp.cxx:1.7, (b) was easy to either fix the offending #if FOO to something like #ifdef FOO (which appeared to be the intended code, and the original author had merely been sloppy), or (c) was easy to wrap the offending (external) header in some disable-warnings #pragma (which was needed anyway, as the external header also produced other warnings). Thus, I am reluctant to disable this warning globally. - unfortunately, it's implicitely relied upon by boost's BOOST_WORKAROUND macro (boost/workaround.hpp). Since no other compiler warns about that, I've no idea how to fix BOOST_WORKAROUND, and this macro's use is quite abundant in boost, I'd also suggest disabling it. This could be temporary, until upstream boost has fixed/workarounded this. Would it work to disable that warning locally within boost headers? Last, but not least there's a really annoying side effect of gcc's -Wshadow warning: when declaring functions that overload one of the gcc builtin functions, you get the following warning issued: stl/stl/_complex.h:782: warning: shadowing built-in function `int std::abs(int)' Switching off -Wshadow cures this, as well as disabling gcc's builtins via -fno-builtin. But both prolly aren't changes we want to do persistently for gcc. Any other ideas? We already sprinkled our STLport patches with pragmas to disable warnings locally within those headers. Seems this is just yet another place where we need to do that. -Stephan Thanks for listening, feedback greatly appreciated, - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: some more warnings that might be disabled
Stephan Bergmann [EMAIL PROTECTED] writes: Thorsten Behrens wrote: - Compiler Warning (level 3) C4686 'user-defined type' : possible change in behavior, change in UDT return calling convention [...] Agreed that such warnings should be disabled globally. (I also want to disable warning C4347: behavior change: 'overload A' is called instead of 'overload B' on CWS sb59, which is for example triggered by some instances of std::minsal_Int32(a,b).) Ok - will disable it then. C4668 can potentially unearth real portability problems (that a try to access an undefined preprocessor macro evaluates to '0' is a convention possibly not shared by all c++ compilers of this universe) This behavior is not a convention, but mandated by the C/C++ standards. Still, in all the cases I encountered so far where MSC issued this warning, it either (a) unearthed really broken code like #if STRCODE==sal_Unicode in tools/source/string/strimp.cxx:1.7, (b) was easy to either fix the offending #if FOO to something like #ifdef FOO (which appeared to be the intended code, and the original author had merely been sloppy), or (c) was easy to wrap the offending (external) header in some disable-warnings #pragma (which was needed anyway, as the external header also produced other warnings). Thus, I am reluctant to disable this warning globally. - unfortunately, it's implicitely relied upon by boost's BOOST_WORKAROUND macro (boost/workaround.hpp). Since no other compiler warns about that, I've no idea how to fix BOOST_WORKAROUND, and this macro's use is quite abundant in boost, I'd also suggest disabling it. This could be temporary, until upstream boost has fixed/workarounded this. Would it work to disable that warning locally within boost headers? In principle, yes - only that there are currently ~25 files to clobber with this (but see below). Last, but not least there's a really annoying side effect of gcc's -Wshadow warning: when declaring functions that overload one of the gcc builtin functions, you get the following warning issued: stl/stl/_complex.h:782: warning: shadowing built-in function `int std::abs(int)' Switching off -Wshadow cures this, as well as disabling gcc's builtins via -fno-builtin. But both prolly aren't changes we want to do persistently for gcc. Any other ideas? We already sprinkled our STLport patches with pragmas to disable warnings locally within those headers. Seems this is just yet another place where we need to do that. Hm. I'm not sure whether this is an approach that scales. Admittedly, the ~180 cases in STLport-4.0.patch are generated automatically - but is anything of that going to be upstreamed? There's of course a fine line between fixing obviously incorrect code, and mutilating it to the likings of specific compiler's warning sets. The latter does not make the code more portable, and significantly increases the chance of build breakage on the platforms not built (unless there are zero-cost mechanisms to build on _all_ werror=t platforms - c.f. build bots). I'm not suggesting that C4668 falls into this category (-Wshadow clearly does not), but I'm convinced that we must agree on a set of warnings that's enforced (werror=t), and a superset that can optionally be used by the developer to improve code quality. For the former, we should err on the safe side, the latter could also contain more controversial stuff (I'm still a big fan of gcc's -Weffc++). Would you agree that having a warning enabled that has uncovered one real bug in N years of development, offers a bad cost/benefit ratio if it needs recurring fixes in external code/frequently breaks HEAD? Cheers, -- Thorsten If you're not failing some of the time, you're not trying hard enough. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
[dev] Warning free code: some more warnings that might be disabled
Hi folks, while trying to get basebmp and vigra warning free, I've been bitten by the following two msvc warnings: - Compiler Warning (level 3) C4686 'user-defined type' : possible change in behavior, change in UDT return calling convention A class template specialization was not is defined before it was used in a return type. Anything that instantiates the class will resolve C4686; declaring an instance or accessing a member (Cint::anything) are also options. - Compiler Warning (level 4) C4668 Error Message 'symbol' is not defined as a preprocessor macro, replacing with '0' for 'directives' A symbol that was not defined was used with a preprocessor directive. The symbol will evaluate to false. To define a symbol, you can use either the #define directive or /D compiler option. - C4686 is one of those infamous 'let's warn our users, because we've changed behaviour and fixed a nonconformant behaviour' MS noise generators (it's about which calling convention is used for the not-yet-instantiated return type). I'd say could safely be disabled. C4668 can potentially unearth real portability problems (that a try to access an undefined preprocessor macro evaluates to '0' is a convention possibly not shared by all c++ compilers of this universe) - unfortunately, it's implicitely relied upon by boost's BOOST_WORKAROUND macro (boost/workaround.hpp). Since no other compiler warns about that, I've no idea how to fix BOOST_WORKAROUND, and this macro's use is quite abundant in boost, I'd also suggest disabling it. This could be temporary, until upstream boost has fixed/workarounded this. Last, but not least there's a really annoying side effect of gcc's -Wshadow warning: when declaring functions that overload one of the gcc builtin functions, you get the following warning issued: stl/stl/_complex.h:782: warning: shadowing built-in function `int std::abs(int)' Switching off -Wshadow cures this, as well as disabling gcc's builtins via -fno-builtin. But both prolly aren't changes we want to do persistently for gcc. Any other ideas? Thanks for listening, feedback greatly appreciated, -- Thorsten If you're not failing some of the time, you're not trying hard enough. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
[dev] Bad operator overloading, was: Re: [dev] Warning free code: the missing modules
void f(com::sun::star::uno::Any const a) {
sal_Bool b;
if (a = b) {
// warning if b is used here
}
}
Ok, I'm not a C++ programmer and this is probably not the right place
to start learning.
But isn't it the left (a) side that gets assigned in a = b?
I.e. b is still not assigned and the warning is good and fixing it is
no mutilation?
If both a and b were built in types, then yes, the used = would assign
to a, not b. However, here (as a is of type com::sun::star::uno::Any) a
user-defined operator = is used, which happens to have rather odd
semantics (in that it takes its first argument as const reference and
instead assigns to the second argument).
Which is a violation of OOo coding guidelines and insofar a bug.
Exchanging the assigned-to-partner in a ...= operator is too
misleading IMO, even for stream operators.
So the warning indeed showed a case. The compiler is not able to know,
that b got a value assigned, because normal operator syntax was exchanged.
Unfortunately this probably is not fixable, because of API stability. Or
is it?
Nikolai
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
The choice of operator is indeed unfortunate. However, I do not agree that - T b; + T b = T(); is in general a fix that improves code quality. I beg to disagree. It is always possible that between declaration and first use of b other code is added later. If then b has at least a defined value (though not a previously used one), error tracking is easier. Nikolai - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Nikolai Pretzell schrieb: The choice of operator is indeed unfortunate. However, I do not agree that - T b; + T b = T(); is in general a fix that improves code quality. I beg to disagree. It is always possible that between declaration and first use of b other code is added later. If then b has at least a defined value (though not a previously used one), error tracking is easier. To be precise, of course T() may not be always the right decision. I only remarked to leaving it un-initialized. Nikolai - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Kay Ramme - Sun Germany - Hamburg wrote: Stephan Bergmann wrote: Kay Ramme - Sun Germany - Hamburg wrote: Hi, please correct me, if I am wrong. I understand this as a 'C' inherited C++ oddity (no constructors for integral values), which leads to a warning if the first operation on the integral value is not classified as assignment. Obviously 'operator =' has not been classified as assignment at least not for the right operand. No, the compiler does not assume the user-supplied operator = has any assignment semantics. Rather, as the operator = is inline, GCC tries I thought that this is what I said ;-), however. So, it seems that we have used the wrong operator here. Therefor I tend to agree to Frank, that we may want to fix this. The choice of operator is indeed unfortunate. However, I do not agree that - T b; + T b = T(); is in general a fix that improves code quality. Certainly not in general, only for the places the warning gets emitted. And only for the reason, that we may have chosen the wrong operator. I do not understand why the choice of function name should motivate us to change or not to change some code so that the (imperfect) definitive-assignment-analysis of some compiler does not cause problems. (The chosen function name operator = is IMO unfortunate in that a human reader might assume modification of the lhs, whereas it actually does assignment to the rhs.) If I get it right, the compiler makers are more or less in a dilemma. Why? They just need to be conservative in definitive-assignment-analysis and do not emit warnings if the compiler is not sure that a variable has definitely not been assigned a value (as most compiler versions seem to do). -Stephan -Stephan Kay - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Frank Schönheit - Sun Microsystems Germany wrote: Hi Stephan, So, it seems that we have used the wrong operator here. Therefor I tend to agree to Frank, that we may want to fix this. The choice of operator is indeed unfortunate. However, I do not agree that - T b; + T b = T(); is in general a fix that improves code quality. Don't know which Ts are affected, but the original example was about sal_Bool, and - sal_Bool b; + sal_Bool b( sal_False ); seems legitimate to me. With GCC 4.0.2, sal_Bool and other small integral types like sal_Int16, sal_Unicode are affected. With GCC 4.1.1, only those other small integral types are affected (which are used considerably less often in conjunction with com::sun::star::Any than sal_Bool). In the case of sal_Bool, it might be true that false is a better default value than true (no pun intended), but that need not be the case always, so you would have to analyze each place in the code where you add = false, which is tedious due to the large number of places involved. And still, I personally consider it a degradation of code quality to explicitly assign an (arbitrary) default value to a variable in places where it is obvious that the variable is only used after definitively having been assigned to (in places where that is not obvious, I think it is often better to restructure the code to make that obvious, than to initially assign an arbitrary dummy value to the variable; rationale: in the latter case, the reader has to scan the code to find out whether the initial assignment is of importance, or merely there to silence warnings from the compiler). Besides that: Sure, not every change we did during the warning-freeness ride does directly improve code quality. But this is not the point - it's getting the code to compile without warning :) ...but only on compilers where we can achieve this with a reasonable amount of work. If we would need to make lots of hacky changes to make a single version of GCC work, while most other versions (even more recent ones) of GCC would not need that, I think we should weigh what is more important. That's all I wanted to say. There will probably always be warnings which are wrong. Remember for instance MSVC's Foo* pFoo = ...; pFoo-doSomething(); which sometimes yields the wrong pFoo might be used without being initialized (or so) warning. In those cases, we simply need to workaround the compiler bug/limitation here. Of course, if there are other alternatives (other than disabling the respective warning completely), then that's fine, too. However, none springs to my mind ... Not supporting GCC 4.0.2 is the alternative that springs to my mind (see above). -Stephan Ciao Frank - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Hi Stephan, In the case of sal_Bool, it might be true that false is a better default value than true (no pun intended), but that need not be the case always, so you would have to analyze each place in the code where you add = false, which is tedious due to the large number of places involved. Sure, this analysis has to take place. forgive me if I missed this in the thread: About which order of magnitued do we talk here? 10 places? 1? And still, I personally consider it a degradation of code quality to explicitly assign an (arbitrary) default value to a variable in places where it is obvious that the variable is only used after definitively having been assigned to (in places where that is not obvious, I think it is often better to restructure the code to make that obvious, than to initially assign an arbitrary dummy value to the variable; rationale: in the latter case, the reader has to scan the code to find out whether the initial assignment is of importance, or merely there to silence warnings from the compiler). Well, the warning could be an indicator to think about whether restructuring would be a good idea here ... That's the basic idea behind warnings, isn't it? Sometimes, they point to an error - then fix it. Sometimes, they don't, then check whether you can write code which doesn't expose the warning. This does not always mean silence the warning by brute force (which is assigning a default in our case), but restructure your code so it becomes better and less warning-prone is of course always a valid alternative :) ...but only on compilers where we can achieve this with a reasonable amount of work. If we would need to make lots of hacky changes to make a single version of GCC work, while most other versions (even more recent ones) of GCC would not need that, I think we should weigh what is more important. That's all I wanted to say. Okay, I agree. As said, I might have missed the number of places affected. For a certain amount, I'd probably also say it's not worth it ... (though my gut feeling is that my limit would be higher than your's here :) Not supporting GCC 4.0.2 is the alternative that springs to my mind (see above). Okay, cannot judge how many people will be repelled by this. But if it's as buggy as you say, this might be no problem, anyway ... Ciao Frank -- - Frank Schönheit, Software Engineer [EMAIL PROTECTED] - - Sun Microsystems http://www.sun.com/staroffice - - OpenOffice.org Database http://dba.openoffice.org - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Frank Schönheit - Sun Microsystems Germany wrote: Hi Stephan, In the case of sal_Bool, it might be true that false is a better default value than true (no pun intended), but that need not be the case always, so you would have to analyze each place in the code where you add = false, which is tedious due to the large number of places involved. Sure, this analysis has to take place. forgive me if I missed this in the thread: About which order of magnitued do we talk here? 10 places? 1? 10^2--10^3 would be my guess -Stephan [...] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Hi Stephan, forgive me if I missed this in the thread: About which order of magnitued do we talk here? 10 places? 1? 10^2--10^3 would be my guess Hmm, that's quite a lot ... Ciao Frank -- - Frank Schönheit, Software Engineer [EMAIL PROTECTED] - - Sun Microsystems http://www.sun.com/staroffice - - OpenOffice.org Database http://dba.openoffice.org - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Christian Lohmaier wrote:
Hi *,
On Fri, Jul 14, 2006 at 10:00:57AM +0200, Stephan Bergmann wrote:
Christian Lohmaier wrote:
On Thu, Jul 13, 2006 at 06:04:41PM +0200, Stephan Bergmann wrote:
Now that CWS warnings01 is integrated and most of the OOo modules are
C/C++ warning free for the standard platforms (unxlngi6, unxsoli4,
unxsols4, wntmsci10),
only for specific compiler versions.
gcc 4.0.[23] at least cannot compile OOo without disabling the warnings
for quite a lot of dirs.
Please wait until CWS warningfixes02 is integrated, or manually apply
the patch from http://www.openoffice.org/issues/show_bug.cgi?id=66577.
I already have that one applied, but still, many, many directories need
the warnings != errors switch. (around 90)
With those changes integrated, OOo should compile fine at least with
GCC 4.1.1. I encountered spurious warnings with GCC 4.0.2 that I would
consider errors of GCC, and which disappeared for GCC 4.1.1. This might
rule out GCC 4.0.2 as an appropriate compiler for OOo.
Unfortunately :-(
[...]
Which modules are missing? As of SRC680m176, the situation is as follows:
m175 and m176 don't compile here either because typesizes.h is not
generated/ typesconfig is not built and not run. (but build walked
through the directory) - not sure why But that's another story
anyway.
You mean, unrelated to the warnings are errors thing? (Otherwise, I
would be interested in an issue id.)
Not related to the warnings are errors thing. I now tracked it down to a
broken patch that I applied
(http://qa.openoffice.org/issues/show_bug.cgi?id=67199 - I thought I had
the latest one, but my scripts applied the parallel-solenv-urd.diff one
instead)...
Opinions, anyone?
I'd like to have a statement of what compiler versions will be/are still
supported by OOo.
The official Sun Hamburg builds are done with GCC 3.4.1 (plus a
visibility patch and one other minor tweak, I think). Other versions
are also known to work, either by chance, or after some people put in
enough effort to make it work. Other versions are known not to work,
either due to errors in GCC or in OOo (wasn't that the case for GCC
4.0.0? I'm always bad at remembering such details).
I'm feared but nevertheless expected the by chance answer...
I personally would love to see as many versions of GCC supported as
possible, but there are certainly practical limits to this.
So in future, the only way to build with those other compiler versions
is to either patch a lot of makefiles or to disable the warnings=errors
flag completely?
That each person that wants to build OOo with compiler xy has to patch
a lot of makefiles IMO is no solution. Either OOo builds on copiler xy
out of the box or not at all. Also, I'm reluctant to introduce a global
switch that undoes warnings=errors for a build. I fear that switch
would be used too often to cheaply work around serious problems. New
compilers with new kinds of warnings can give us valuable information
about where our code is broken. Of course, it is also true that new
compilers can start to issue bogus warnings that cause endless pain for
those trying to use those compilers...
Yes, introduction of warnings=errors changed the landscape somewhat.
Now, it is no longer only (bogus) compilation errors or generation of
wrong code that can rule out a specific compiler, but also (bogus)
compilation warnings. I would hope that for each given compiler, we can
either change the OOo code base so that it compiles without warnings (as
was done e.g. for GCC 4.1.1) or decide that the compiler is not up to
the task (as I would suggest for GCC 4.0.2, but that is only my personal
opinion, see below). However, when changing the OOo code base so that
it compiles fine with a certain compiler, there is a line somewhere
between improving the code so that it does not cause warnings and
brutally mutilating the code to adapt it to the idiosyncrasies of that
compiler. Before we ever fall into the trap of doing the latter, we
should seriously re-consider whether supporting that specific compiler
version is worth it---maintaining a contorted code base is too high a
cost overall.
As you wrote in comment 3 to issue 66577 that gcc 4.0 generates warnings
or stuff like this
void f(com::sun::star::uno::Any const a) {
sal_Bool b;
if (a = b) {
// warning if b is used here
}
}
To me (I have no programming skills), this looks like a warning similar
to you'd better put parentheses around that expression to avoid
ambiguity (You'd better set a value for b or you may get weired
results) - but I cannot tell whether that is worth fixing or not.
It is not about parentheses but about definitive assignment of b. The
fix would be sal_Bool b = false; instead of sal_Bool b;, and IMO
that starts to touch the line between improvement and mutilation. A
later reader of the code might be puzzled why b is first assigned a
dummy value when all the uses of b
Re: [dev] Warning free code: the missing modules
2006/7/18, Stephan Bergmann [EMAIL PROTECTED]:
As you wrote in comment 3 to issue 66577 that gcc 4.0 generates warnings
or stuff like this
void f(com::sun::star::uno::Any const a) {
sal_Bool b;
if (a = b) {
// warning if b is used here
}
}
To me (I have no programming skills), this looks like a warning similar
to you'd better put parentheses around that expression to avoid
ambiguity (You'd better set a value for b or you may get weired
results) - but I cannot tell whether that is worth fixing or not.
It is not about parentheses but about definitive assignment of b. The
fix would be sal_Bool b = false; instead of sal_Bool b;, and IMO
that starts to touch the line between improvement and mutilation. A
later reader of the code might be puzzled why b is first assigned a
dummy value when all the uses of b are within the if block where it
definitely is assigned a value.
Ok, I'm not a C++ programmer and this is probably not the right place
to start learning.
But isn't it the left (a) side that gets assigned in a = b?
I.e. b is still not assigned and the warning is good and fixing it is
no mutilation?
/$
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Hi, please correct me, if I am wrong. I understand this as a 'C' inherited C++ oddity (no constructors for integral values), which leads to a warning if the first operation on the integral value is not classified as assignment. Obviously 'operator =' has not been classified as assignment at least not for the right operand. So, it seems that we have used the wrong operator here. Therefor I tend to agree to Frank, that we may want to fix this. Kay Frank Schönheit - Sun Microsystems Germany wrote: Nonetheless, I would not consider initializing b with something meaningful a mutilation. And be it just because months later, somebody will be tempted to re-use b some lines below. Ciao Frank - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Kay Ramme - Sun Germany - Hamburg wrote:
Hi,
please correct me, if I am wrong. I understand this as a 'C' inherited
C++ oddity (no constructors for integral values), which leads to a
warning if the first operation on the integral value is not classified
as assignment. Obviously 'operator =' has not been classified as
assignment at least not for the right operand.
No, the compiler does not assume the user-supplied operator = has any
assignment semantics. Rather, as the operator = is inline, GCC tries
an analysis whether or not b is definitely assigned at point (1) in
com::sun::star::uno::Any a;
T b;
if (a = b) {
// (1)
}
Depending on T (i.e., depending on the complexity of the relevant inline
operator =):
- GCC sometimes comes up with the insight that b is definitely assigned
at (1), which is correct, and in which case GCC does not issue a warning
(why should it?);
- GCC sometimes cannot decide whether or not b is definitely assigned at
(1), although it *is*, and in which case GCC conservatively does not
issue a warning;
- GCC sometimes comes up with the insight that b is not definitely
assigned at (1), which is an incorrect analysis, and in which case GCC
erroneously issues a warning.
So, it seems that we have used the wrong operator here. Therefor I tend
to agree to Frank, that we may want to fix this.
The choice of operator is indeed unfortunate. However, I do not agree that
- T b;
+ T b = T();
is in general a fix that improves code quality.
-Stephan
Kay
Frank Schönheit - Sun Microsystems Germany wrote:
Nonetheless, I would not consider initializing b with something
meaningful a mutilation. And be it just because months later, somebody
will be tempted to re-use b some lines below.
Ciao
Frank
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Stephan Bergmann wrote: [...] C2 (M2, M3, M4): Create one CWS for the 20 wntmsci10 modules, svx, and desktop. Testing will be somewhat expensive. We should start this early and see we can integrate early in 2.0.5 timeframe. This could be distributed between three persons, one each for M2--M4. (Any volunteers?) I just started CWS sb59 for this. For the wntmsci10 modules, I will see how far I get on my own before spreading the work (as those modules are already warning-free for Linux/Solaris, there should not be too much to do). For svx, Christian Lippka will coordinate who fixes what. -Stephan [...] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Stephan Bergmann wrote: Kay Ramme - Sun Germany - Hamburg wrote: Hi, please correct me, if I am wrong. I understand this as a 'C' inherited C++ oddity (no constructors for integral values), which leads to a warning if the first operation on the integral value is not classified as assignment. Obviously 'operator =' has not been classified as assignment at least not for the right operand. No, the compiler does not assume the user-supplied operator = has any assignment semantics. Rather, as the operator = is inline, GCC tries I thought that this is what I said ;-), however. So, it seems that we have used the wrong operator here. Therefor I tend to agree to Frank, that we may want to fix this. The choice of operator is indeed unfortunate. However, I do not agree that - T b; + T b = T(); is in general a fix that improves code quality. Certainly not in general, only for the places the warning gets emitted. And only for the reason, that we may have chosen the wrong operator. If I get it right, the compiler makers are more or less in a dilemma. -Stephan Kay - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Hi Stephan, So, it seems that we have used the wrong operator here. Therefor I tend to agree to Frank, that we may want to fix this. The choice of operator is indeed unfortunate. However, I do not agree that - T b; + T b = T(); is in general a fix that improves code quality. Don't know which Ts are affected, but the original example was about sal_Bool, and - sal_Bool b; + sal_Bool b( sal_False ); seems legitimate to me. Besides that: Sure, not every change we did during the warning-freeness ride does directly improve code quality. But this is not the point - it's getting the code to compile without warning :) There will probably always be warnings which are wrong. Remember for instance MSVC's Foo* pFoo = ...; pFoo-doSomething(); which sometimes yields the wrong pFoo might be used without being initialized (or so) warning. In those cases, we simply need to workaround the compiler bug/limitation here. Of course, if there are other alternatives (other than disabling the respective warning completely), then that's fine, too. However, none springs to my mind ... Ciao Frank -- - Frank Schönheit, Software Engineer [EMAIL PROTECTED] - - Sun Microsystems http://www.sun.com/staroffice - - OpenOffice.org Database http://dba.openoffice.org - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Bjoern Milcke wrote: Hi Stephan, I can add EXTERNAL_WARNINGS_NOT_ERRORS=TRUE to every makefile.mk in sch on CWS sb58 to effectively exempt it from warning-freedom, just as I did for binfilter. Yes, please. testhxx only found (very few) problems in memchrt.hxx, and only on wntmsci10[.pro], so getting those headers into shape so that they do not break sc/sd/sw is hopefully no big deal (and can probably done in one of the CWS for sc/sd/sw?). That sounds good. The CWS to fix this in could be the same where the variables in makefile.mk are added, so we have only one warnings-CWS that contains sch. As stated above, I already added modification of the sch makefile.mks to sb58 (simply because I had the awk script for doing the same to binfilter still lying around), but I would like to keep sb58 a no-real-changes-to-the-code CWS (so that it does not need testing). Hence, any changes to sch/memchrt.hxx will need to go into another CWS. -Stephan -Bjoern - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Hi *,
On Fri, Jul 14, 2006 at 10:00:57AM +0200, Stephan Bergmann wrote:
Christian Lohmaier wrote:
On Thu, Jul 13, 2006 at 06:04:41PM +0200, Stephan Bergmann wrote:
Now that CWS warnings01 is integrated and most of the OOo modules are
C/C++ warning free for the standard platforms (unxlngi6, unxsoli4,
unxsols4, wntmsci10),
only for specific compiler versions.
gcc 4.0.[23] at least cannot compile OOo without disabling the warnings
for quite a lot of dirs.
Please wait until CWS warningfixes02 is integrated, or manually apply
the patch from http://www.openoffice.org/issues/show_bug.cgi?id=66577.
I already have that one applied, but still, many, many directories need
the warnings != errors switch. (around 90)
With those changes integrated, OOo should compile fine at least with
GCC 4.1.1. I encountered spurious warnings with GCC 4.0.2 that I would
consider errors of GCC, and which disappeared for GCC 4.1.1. This might
rule out GCC 4.0.2 as an appropriate compiler for OOo.
Unfortunately :-(
[...]
Which modules are missing? As of SRC680m176, the situation is as follows:
m175 and m176 don't compile here either because typesizes.h is not
generated/ typesconfig is not built and not run. (but build walked
through the directory) - not sure why But that's another story
anyway.
You mean, unrelated to the warnings are errors thing? (Otherwise, I
would be interested in an issue id.)
Not related to the warnings are errors thing. I now tracked it down to a
broken patch that I applied
(http://qa.openoffice.org/issues/show_bug.cgi?id=67199 - I thought I had
the latest one, but my scripts applied the parallel-solenv-urd.diff one
instead)...
Opinions, anyone?
I'd like to have a statement of what compiler versions will be/are still
supported by OOo.
The official Sun Hamburg builds are done with GCC 3.4.1 (plus a
visibility patch and one other minor tweak, I think). Other versions
are also known to work, either by chance, or after some people put in
enough effort to make it work. Other versions are known not to work,
either due to errors in GCC or in OOo (wasn't that the case for GCC
4.0.0? I'm always bad at remembering such details).
I'm feared but nevertheless expected the by chance answer...
I personally would love to see as many versions of GCC supported as
possible, but there are certainly practical limits to this.
So in future, the only way to build with those other compiler versions
is to either patch a lot of makefiles or to disable the warnings=errors
flag completely?
As you wrote in comment 3 to issue 66577 that gcc 4.0 generates warnings
or stuff like this
void f(com::sun::star::uno::Any const a) {
sal_Bool b;
if (a = b) {
// warning if b is used here
}
}
To me (I have no programming skills), this looks like a warning similar
to you'd better put parentheses around that expression to avoid
ambiguity (You'd better set a value for b or you may get weired
results) - but I cannot tell whether that is worth fixing or not.
From your comment I guess you don't think it is worth fixing.
(And if it is not worth fixing, would declaring it do harm?
You had some lines for fixing these kind of warnings in your
firsthalf.diff, but removed them from your second patch, that's why I'm
asking)
[...]
ciao
Christian
--
NP: Soulfly - Soulfly II
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Hi Stephan, Opinions, anyone? sounds good, except that I would not give complete M2, M3, M4 to one person each. Those are huge tasks, and touch code where you sometimes better know what you're doing, so I suggest to do like before: Let everybody fix his/her own code here. Ciao Frank -- - Frank Schönheit, Software Engineer [EMAIL PROTECTED] - - Sun Microsystems http://www.sun.com/staroffice - - OpenOffice.org Database http://dba.openoffice.org - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Christian Lohmaier wrote: On Thu, Jul 13, 2006 at 06:04:41PM +0200, Stephan Bergmann wrote: Now that CWS warnings01 is integrated and most of the OOo modules are C/C++ warning free for the standard platforms (unxlngi6, unxsoli4, unxsols4, wntmsci10), only for specific compiler versions. gcc 4.0.[23] at least cannot compile OOo without disabling the warnings for quite a lot of dirs. Please wait until CWS warningfixes02 is integrated, or manually apply the patch from http://www.openoffice.org/issues/show_bug.cgi?id=66577. With those changes integrated, OOo should compile fine at least with GCC 4.1.1. I encountered spurious warnings with GCC 4.0.2 that I would consider errors of GCC, and which disappeared for GCC 4.1.1. This might rule out GCC 4.0.2 as an appropriate compiler for OOo. [...] Which modules are missing? As of SRC680m176, the situation is as follows: m175 and m176 don't compile here either because typesizes.h is not generated/ typesconfig is not built and not run. (but build walked through the directory) - not sure why But that's another story anyway. You mean, unrelated to the warnings are errors thing? (Otherwise, I would be interested in an issue id.) Opinions, anyone? I'd like to have a statement of what compiler versions will be/are still supported by OOo. The official Sun Hamburg builds are done with GCC 3.4.1 (plus a visibility patch and one other minor tweak, I think). Other versions are also known to work, either by chance, or after some people put in enough effort to make it work. Other versions are known not to work, either due to errors in GCC or in OOo (wasn't that the case for GCC 4.0.0? I'm always bad at remembering such details). I personally would love to see as many versions of GCC supported as possible, but there are certainly practical limits to this. Thus, in short: If you are looking for a hassle-free build, go with GCC 3.4.1 for now. If you want something else, be prepared that there might be problems lurking in every single build (from trivial to show-stopper), but also be assured that the rest of the OOo community is eager to remove those problems, too. -Stephan ciao Christian - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Stephan Bergmann wrote: [...] M7 basctl, chart2, extensions, filter, lingu, xmlsecurity, starmath, sc, sd, slideshow, sw, writerperfect. All depending on svx; additionally, sd is depending on slideshow and the canvas stuff above, and sw is depending on writerperfect. What I forgot to mention: sc, sd, sw also depend on two headers delivered from sch, sch/memchrt.hxx and sch/schdll.hxx. A quick check with testhxx shows that sch/memchrt.hxx needs to be made warning-free on wntmsci10[.pro]. [...] -Stephan - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Hi Stephan, M8 sch. Depending on svx. Do we want to make it warning clean, or do we want to wait for chart2? IMO making sch warning free is a complete waste of time, as it will be replaced. All time we invest there is missing for developing chart2. As you also mentioned the delivered headers memchrt.hxx and schdll.hxx, those probably must be made warning-free to avoid warnings in the projects that include them. I think schdll.hxx is not very complex and similar to scdll.hxx, sddll.hxx etc., so fixing this might be no problem (you never know, though ;-) ) For memchrt.hxx the question remains how much effort it would be to make it warning-free. It contains a lot of inline-code. I suppose there is no option to ignore warnings for a single file (like preprocessor directives)? Regards, Bjoern - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Bjoern Milcke wrote: Hi Stephan, M8 sch. Depending on svx. Do we want to make it warning clean, or do we want to wait for chart2? IMO making sch warning free is a complete waste of time, as it will be replaced. All time we invest there is missing for developing chart2. I can add EXTERNAL_WARNINGS_NOT_ERRORS=TRUE to every makefile.mk in sch on CWS sb58 to effectively exempt it from warning-freedom, just as I did for binfilter. As you also mentioned the delivered headers memchrt.hxx and schdll.hxx, those probably must be made warning-free to avoid warnings in the projects that include them. I think schdll.hxx is not very complex and similar to scdll.hxx, sddll.hxx etc., so fixing this might be no problem (you never know, though ;-) ) For memchrt.hxx the question remains how much effort it would be to make it warning-free. It contains a lot of inline-code. I suppose there is no option to ignore warnings for a single file (like preprocessor directives)? testhxx only found (very few) problems in memchrt.hxx, and only on wntmsci10[.pro], so getting those headers into shape so that they do not break sc/sd/sw is hopefully no big deal (and can probably done in one of the CWS for sc/sd/sw?). -Stephan Regards, Bjoern - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Hi Stephan, I can add EXTERNAL_WARNINGS_NOT_ERRORS=TRUE to every makefile.mk in sch on CWS sb58 to effectively exempt it from warning-freedom, just as I did for binfilter. Yes, please. testhxx only found (very few) problems in memchrt.hxx, and only on wntmsci10[.pro], so getting those headers into shape so that they do not break sc/sd/sw is hopefully no big deal (and can probably done in one of the CWS for sc/sd/sw?). That sounds good. The CWS to fix this in could be the same where the variables in makefile.mk are added, so we have only one warnings-CWS that contains sch. -Bjoern - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
[dev] Warning free code: the missing modules
Hi again, Now that CWS warnings01 is integrated and most of the OOo modules are C/C++ warning free for the standard platforms (unxlngi6, unxsoli4, unxsols4, wntmsci10), it is time to address the remaining modules on those standard platforms. I think we (re-?)learned two lessons from CWS warnings01: One large CWS is not really tractable; and doing the changes in a closed style and only dumping the results into the larger community leads to problems due to the community's compiler/baseline diversity. We should keep that in mind when going forward. Which modules are missing? As of SRC680m176, the situation is as follows: M1 I just created CWS sb58 to get rid of false positives (i.e., modules that are listed in some MODULES_WITH_WARNINGS list but do not contain any C/C++ code at all) and those modules that we do not want to clean up but instead compile with EXTERNAL_WARNINGS_NOT_ERRORS=TRUE (binfilter and maybe sch). This CWS will not change any code, but only make sure appropriate warning switches are used during compilation. M2 On wntmsci10, some 20 modules are not yet cleaned up that are already cleaned up for the other platforms (mostly between vcl and toolkit within the hierarchy). M3 svx is only half-finished. M4 desktop. M5 devtools, r_tools, b_server. M6 canvas (only finished on unxlngi6), cppcanvas, dxcanvas, glcanvas. M7 basctl, chart2, extensions, filter, lingu, xmlsecurity, starmath, sc, sd, slideshow, sw, writerperfect. All depending on svx; additionally, sd is depending on slideshow and the canvas stuff above, and sw is depending on writerperfect. M8 sch. Depending on svx. Do we want to make it warning clean, or do we want to wait for chart2? When grouping these into CWSs, we should take into account dependencies and testing: Modules like svx impact much of OOo's behavior, so the easiest way to test excessive changes in such modules can be to do exhaustive automated tests across the board (as have been done for CWS warnings01), and then it probably makes sense to group those modules together in one CWS, so that the large number of tests needs only be done once. I would thus suggest the following strategy: C1 (M1, M8): CWS sb58 is already in progress by me, should be ready for 2.0.4, no real testing needed. C2 (M2, M3, M4): Create one CWS for the 20 wntmsci10 modules, svx, and desktop. Testing will be somewhat expensive. We should start this early and see we can integrate early in 2.0.5 timeframe. This could be distributed between three persons, one each for M2--M4. (Any volunteers?) C3 (M5): Create one CWS for devtools, r_tools, b_server. This depends on C2, but is otherwise independent of the rest (i.e., could be started in 2.0.5 or later). C4 (M6): Create one CWS for canvas, cppcanvas, dxcanvas, glcanvas. This depends on C2, and the sd CWS will depend on it (i.e., should be started in 2.0.5). C5--C14 (M7): Create one CWS each for C5 basctl (depends on C2) C6 chart2 (depends on C2) C7 extensions (depends on C2) C8 filter (depends on C2) C9 lingu (depends on C2) C10 xmlsecurity (depends on C2) C11 starmath (depends on C2) C12 sc (depends on C2) C13 sd, slideshow (depends on C2 and C4) C14 sw, writerperfect (depends on C2) (i.e., can probably be started for 2.0.5/2.0.6). Opinions, anyone? -Stephan - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
Stephan Bergmann [EMAIL PROTECTED] writes: C4 (M6): Create one CWS for canvas, cppcanvas, dxcanvas, glcanvas. This depends on C2, and the sd CWS will depend on it (i.e., should be started in 2.0.5). Fine with me. Cheers, -- Thorsten If you're not failing some of the time, you're not trying hard enough. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning free code: the missing modules
On Thu, Jul 13, 2006 at 06:04:41PM +0200, Stephan Bergmann wrote: Now that CWS warnings01 is integrated and most of the OOo modules are C/C++ warning free for the standard platforms (unxlngi6, unxsoli4, unxsols4, wntmsci10), only for specific compiler versions. gcc 4.0.[23] at least cannot compile OOo without disabling the warnings for quite a lot of dirs. [...] Which modules are missing? As of SRC680m176, the situation is as follows: m175 and m176 don't compile here either because typesizes.h is not generated/ typesconfig is not built and not run. (but build walked through the directory) - not sure why But that's another story anyway. Opinions, anyone? I'd like to have a statement of what compiler versions will be/are still supported by OOo. ciao Christian -- NP: The White Stripes - My Doorbell - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning-Free Code
http://wiki.services.openoffice.org/twiki/bin/view/Main/WarningFree is a wiki page for all issues related to the Warning-Free Code effort. -Stephan - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Re: sal_IntPtr (was Re: [dev] Warning-Free Code)
Hi Jan, Now I would like to create one more CWS with fixes from ooo64bit02. It will fix occurances of long *pDXArray, which should be sal_Int32 *pDXArray (according to the declaration in VCL). Would you please volunteer to help with review of this one as well? ;-) I volunteer to review these changes... -- Herbert - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Re: sal_IntPtr (was Re: [dev] Warning-Free Code)
Stephan Bergmann wrote: Jan Holesovsky wrote: Hi, On Mon, 29 Aug 2005, Stephan Bergmann wrote: The team members should also have a look at the 64-bit CWS (ooo64bit02) to see what data sizes they already changed. [...] - Added SAL_INT_CAST (for C) and std::static_int_cast and std::reinterpret_int_cast (for C++) to sal/types.h and brought in sal_IntPtr and sal_uIntPtr to sal/types.h from CWS ooo64bit02. See the documentation in sal/types.h for when this new functionality should be used. So, I have extracted all the sal_IntPtr/sal_uIntPtr patches from the ooo64bit02 CWS, Martin Kretzschmar's and my diffs, see the intptr-*.diff's from http://www.go-oo.org/patches/64bit/ . I would like to create a CWS for them, let's say 'intptr' to have them integrated soon, because this is really essential for the 64bit porting. OOo 3.0, the target of 'warnings01', is just too late, and nothing else from 'ooo64bit02' is needed to have this in. Please, who to assign as the QA representative in EIS for this CWS? At the moment I know that the 'intptr' patches build and work OK on x86-64 Linux, and I am in the middle of the x86 Linux build. We'll also have them among the default ooo-build patches to give them proper testing by other people. Thank you, Jan I see you have created an issue in the meantime, that is good. Looking at some of the changes at http://www.go-oo.org/patches/64bit/, there is little overlap with the changes on CWS warnings01. I do not know who would be a good QA person, maybe someone else can comment on that. Jörg Sievers (jsi) agreed to do QA on that CWS, and I would volunteer to do a code review. -Stephan - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning-Free Code
Stephan Bergmann wrote: [...] - Added SAL_INT_CAST (for C) and std::static_int_cast and std::reinterpret_int_cast (for C++) to sal/types.h and brought in sal_IntPtr and sal_uIntPtr to sal/types.h from CWS ooo64bit02. See the documentation in sal/types.h for when this new functionality should be used. Just dropped std::reinterpret_int_cast again (and changed documentation of SAL_INT_CAST to disallow casting between integer and pointer types), as a plain reinterpret_cast is sufficient. The following is a little cookbook of how to cast between pointer and integer types. C++: From pointer-to-(possibly-cv-qualified-)void-or-object type P to unsigned integral type U: P p; U u = sal::static_int_cast U ( reinterpret_cast sal_uIntPtr (p)); From unsigned integral type U to pointer-to-(possibly-cv-qualified-)void-or-object type P: U u; P p = reinterpret_cast P ( sal::static_int_cast sal_uIntPtr (u)); From pointer-to-(possibly-cv-qualified-)void-or-object type P to signed integral type S: P p; S s = sal::static_int_cast S ( reinterpret_cast sal_uIntPtr (p)); From signed integral type S to pointer-to-(possibly-cv-qualified-)void-or-object type P: S s; P p = reinterpret_cast P ( sal::static_int_cast sal_uIntPtr (s)); C: == From pointer-to-(possibly-qualified-)void-or-object type P to unsigned integer type U: P p; U u = SAL_INT_CAST(U, (sal_uIntPtr) p); From unsigned integer type U to pointer-to-(possibly-qualified-)void-or-object type P: U u; P p = (P) SAL_INT_CAST(sal_uIntPtr, u); From pointer-to-(possibly-qualified-)void-or-object type P to signed integer type S: P p; S s = SAL_INT_CAST(S, (sal_uIntPtr) p); From signed integer type S to pointer-to-(possibly-qualified-)void-or-object type P: S s; P p = (P) SAL_INT_CAST(sal_uIntPtr, s); - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning-Free Code
On Fri, 2005-09-02 at 09:58 +0200, Nikolai Pretzell wrote: Hi all, Ken Foskey wrote: So I would actually recommend against an all out warnings push unless everyone is VERY clear the objective is to highlight bugs not to remove warnings. The difference in objectives is very important. Yes, but given the mass of code we have, the only way I see to really highlight bugs, is to remove as much as all warnings. I consider myself a very experienced developer and to boot I have worked through a huge amount of warnings removal in several programs. I have been caught before between the subtle difference between warning removal and removing warnings for the purpose of highlighting bugs. Experience is not the issue, mundane means that even experienced programmers will hide the bug by correcting the warning. The goal of the task is to highlight bugs by removing obvious warnings then taking your time to eliminate the rest. Do this too fast and your fingers and quick solutions will take over. I speak from experience. Thanks Ken Foskey OpenOffice.org Programmer - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning-Free Code
Hi all, Ken Foskey wrote: So I would actually recommend against an all out warnings push unless everyone is VERY clear the objective is to highlight bugs not to remove warnings. The difference in objectives is very important. Yes, but given the mass of code we have, the only way I see to really highlight bugs, is to remove as much as all warnings. 2 To reach that main objective, we have to remove warnings from the current code base. There are to cases: 2a The warning indicates an error in the code. We will fix that error (which is a positive side effect of the efforts spent on this CWS; [...]) 2b The warning is a spurious one. However, to reach the main objective, we have to make it go away anyway, by modifying the code base in some way or other. This (as well as deciding whether the occurence of a warning is case 2a or 2b) is a delicate task, to be sure. I agree with Stephan, that - though this task may be delicate in a few cases (most cases I had a look at are quite simple) - it is necessary. And as the software professionals and/or enthusiasts we are, I don't really think it is impossible. This is just our work. Nikolai - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning-Free Code
Hi,
Ken Foskey wrote:
I am also concerned that then process will become a template fix.
if( fp = fopen( file, r )) {
can become:
if( (fp = fopen( file, r)) ) {
If I assign someone to clean up the error, say a junior programmer
because it is menial, and we have this code:
if( x = 4 ) {
They may very well apply the 'template' solution hiding a useful
warning.
if( (x = 4) ) {
Well, I just assume that the people who do this task are experienced
enough to first understand the code - and only then change it.
- - -
A little sidetrack: I would not suggest to change
if( fp = fopen( file, r )) {
into
if( (fp = fopen( file, r)) ) {,
because the first not only issues a warning, but also is probably not
exception safe.
So rather I'd write:
fp = fopen( file, r );
if(fp != NULL) {
to remove the warning.
And then something similar to
fp = fopen( file, r );
if(fp != NULL) {
FileGuard fg(fp); // will close file in destructor
to ensure exception safety.
Nikolai
That useful warning being removed is WORSE than the problem of many
warnings. This gets really tricky when you get into essoteric C++
warnings.
The objective of the push should be to highlight bugs by removing as
many warnings with obvious solutions as possible. If in doubt leave the
warning!
As Pavel has hinted it is possibly better to work through one warning
class at a time, eg assignment bugs. This can be discussed so that the
approach to each is correct, eg don't just bracket them.
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning-Free Code
Another reason not mentioned is that few compilers complain a lot
about such compact constructs as assignments in if statements, which
leads to another reason why warning-free code is much better (I didn't
see this in the introduction post by Stepahn): What gives warnings on
one Operating System, might *break* on another.
Also I would like to know if Sun Hamburg is interested in warnings
that only come from Mac OS X, or whether community members can work on
that in another cws, etc.?
I ask this because it was said that only Linux, Win32 and Solaris will
be taken care of.
Thanks.
2005/9/2, Nikolai Pretzell [EMAIL PROTECTED]:
[...]
So rather I'd write:
fp = fopen( file, r );
if(fp != NULL) {
to remove the warning.
And then something similar to
fp = fopen( file, r );
if(fp != NULL) {
FileGuard fg(fp); // will close file in destructor
to ensure exception safety.
Nikolai
--
Best Regards
Christian Junker
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning-Free Code
In Message-ID: [EMAIL PROTECTED] Stephan Bergmann [EMAIL PROTECTED] wrote: 3 WE NEED YOU! Here at Hamburg, we will only concentrate on those platforms we build ourselves. Also, due to the fact that our build environment is slightly different from a typical OOo build environment, we may miss some points that will only be noticed by the community. So, we depend on your input for code specific to platforms other than Linux x86, Solaris Intel, Solaris SPARC, and Windows, and for issues that are hidden by our Hamburg build environment. However, as I already wrote above, to keep work on this CWS smooth, please be patient with us and wait until we have reached a state where we can incorporate that input. I will help you a bit, since build will be broken for FreeBSD. how about removing German comments? this is important in two ways: o Many do not understand German :) o some compiler/utilities compilain about non-ASCII character thanks -- NAKATA, Maho ([EMAIL PROTECTED]) - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning-Free Code
Hi Stephan, thanks for that announcement! - Added SAL_INT_CAST (for C) and std::static_int_cast and Here (of course) sal::static_int_cast was meant ... std::reinterpret_int_cast (for C++) and here sal::reinterpret_int_cast. to sal/types.h That has already been correct in the code, this remark only clarifies the posting. Additions to namespace std would be violating the C++ standard. Nikolai - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning-Free Code
Hi michael, michael meeks wrote: On Mon, 2005-08-29 at 22:49 +0200, Pavel Janík wrote: great. Can you do all of your work on 64bit operating systems? ;-) The team members should also have a look at the 64-bit CWS (ooo64bit02) to see what data sizes they already changed. In view of the long-standing non-functioningness of the ooo64bit02 cws - the fact that it breaks 32bit operation ;-), the sheer scale of the problem and the problems of keeping it synchronized with the latest developments - conspire to (in Kendy my opinion[1]) ensure that this 1 massive cws will never be finished, and would be really difficult to test verify - and even harder to get momentum on getting it up-stream at all. Since Heiner is keeping the ooo64bit02 cws update from time to time I'm not aware of problems to keep it up to date with latestet developments, at least not a this time. It makes sense to think about to separate issues from this child workspace to get them integrated earlier and have the fixes available in time so that there would by a synergy between ooo64bit02 and warnings01. In our view it would be far, far better to split up the fixes out of that wherever possible and merge them as parts of other, smaller, incremental CWS' - that can be easily reviewed / shown to be safe, and serve to move the code base to a progressively 64bit-cleaner environment. That would also allow us to start running tinderbox builds over the pieces that are known to work to ensure they don't subsequently break ( at least at compile time ). ie. it'd be great to see people pulling nice looking bits out of 64bit02 and merging them IMHO ;-) If the participants see possibilities to divide the problem in meaningful parts this should be discussed on this list, otherwise we take the risk of uncoordinated and doubled work, HTH, Michael. Martin - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning-Free Code
On Mon, 2005-08-29 at 17:59 +0200, Stephan Bergmann wrote: If someone think it's reasonable to suppress a warning, this must be discussed with the team members. It should also be discussed in public, using the mailing list [EMAIL PROTECTED] Please start the subject with “compiler warnings:”. * Initially I would suggest -Wno-reorder (I think) There are a HUGE number of errors caused simply by the order of the variables in the class declaration and I have only found one instance where it indicated a real error (parent initialisation depended on child data). The needle for this one is small and the haystack particularly large. Because of the pervasiveness of this change merges afterwards a PAINFUL! I speak from experience. I would think about doing this module by module as a separate exercise. * Second possibly -Wno-unknown-pragmas. There are a few pragmas for VC that are in the code and don't really affect things. I have bracketted them but effort for return is fairly low. * Before you start you might want to merge cws waratahbasctl. This is a warnings cws that has lingered for want of a QA person. Have a look at the assignment bug contained within. * I also have a large number of patches that I can apply to save you time. I will also volunteer to assist :-) * In the interim I would recommend adding immediately BY DEFAULT -Wparentheses to the gcc compile. This really does highlight a number of problems with the code, good return error for extra warning ratio. -- Ken Foskey OpenOffice.org developer - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning-Free Code
Ken Foskey wrote: On Mon, 2005-08-29 at 17:59 +0200, Stephan Bergmann wrote: If someone think it's reasonable to suppress a warning, this must be discussed with the team members. It should also be discussed in public, using the mailing list [EMAIL PROTECTED] Please start the subject with “compiler warnings:”. * Initially I would suggest -Wno-reorder (I think) There are a HUGE number of errors caused simply by the order of the variables in the class declaration and I have only found one instance where it indicated a real error (parent initialisation depended on child data). The needle for this one is small and the haystack particularly large. Because of the pervasiveness of this change merges afterwards a PAINFUL! I speak from experience. I would think about doing this module by module as a separate exercise. Just to make sure that I understood correctly: you suggest to ignore these warnings but work on it in a subsequent CWS? This is a real useful warning that can find some really nasty errors, so we should definitely work on keeping it activated, but of course if there are so much places that trigger the warning it could be a good idea to postpone it to a later CWS. Ciao, Mathias -- Mathias Bauer - OpenOffice.org Application Framework Project Lead Please reply to the list only, [EMAIL PROTECTED] is a spam sink. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning-Free Code
On Tue, 2005-08-30 at 18:50 +0200, Mathias Bauer wrote: Ken Foskey wrote: On Mon, 2005-08-29 at 17:59 +0200, Stephan Bergmann wrote: If someone think it's reasonable to suppress a warning, this must be discussed with the team members. It should also be discussed in public, using the mailing list [EMAIL PROTECTED] Please start the subject with “compiler warnings:”. * Initially I would suggest -Wno-reorder (I think) There are a HUGE number of errors caused simply by the order of the variables in the class declaration and I have only found one instance where it indicated a real error (parent initialisation depended on child data). The needle for this one is small and the haystack particularly large. Because of the pervasiveness of this change merges afterwards a PAINFUL! I speak from experience. I would think about doing this module by module as a separate exercise. Just to make sure that I understood correctly: you suggest to ignore these warnings but work on it in a subsequent CWS? This is a real useful warning that can find some really nasty errors, so we should definitely work on keeping it activated, but of course if there are so much places that trigger the warning it could be a good idea to postpone it to a later CWS. Yes, In my experience this warning leads to a lot of changes. Doing this initialy will make the whole process really tedious, (as opposed to simply tedious). It will also break a lot of merges of patches. -- Ken Foskey OpenOffice.org developer - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
[dev] Warning-Free Code
Hi all, Here at Sun Hamburg, we are eager to get OOo's C/C++ code base warning free. We plan to do this massive (in number of files affected) change for post--OOo-2.0. Since doing these changes needs close collaboration (think of headers from one module that only cause warnings once they are used in another module, for example), we initially plan to do this with a Hamburg-only group of engineers, working together on (a single physical copy of) CWS warnings01. If the work becomes too boring for us, we may start to ask for help from the community... ;) This mail contains three sections. In the first section, I repeat the publicly relevant parts of the Hamburg-internal kick-off meeting minutes, written by Malte Timmermann, which put this effort into perspective and give some further details. In the second section, I explain what I have done so far to get CWS warnings01 going, so that the other (Hamburg-internal) engineers can start to do their part of the work. In the third section, I give a quick overview of where we need input from the community. 1 KICK-OFF MEETING MINUTES Reasons for this effort Warning free code is a basic precondition for robust industrial quality code. This as well is current scientific knowledge as the result of the personal experience of Hamburg-internal engineers, as well as the opinion of some OpenOffice.org developers like e.g. Ken Foskey. The main argument against it seems to be, that many warnings are no real bugs, the code works well, and so there would be no time to fix them. Some reasons, why we think it necessary to write warning free code nevertheless: 1. Most warnings are a hint, that either part of the code is not completely thought to the end or something in the depending code has changed or they show inconsistencies. While such occurrences are not always bugs themselves, they show potential problems, that well may occur after the next code change. 2. Warnings are an important source of information. If the mass of warnings is not fixed, the really important ones can not be found in the crowd. 3. A huge number of warnings is an indicator for not robust enough code. 4. We found, that the warnings of gcc give the most positive sensible and the least false positive results. Therefore we choose that as the warning set to orientate for. 5. The daily practice of many software engineers shows that it is quite manageable to write warning free code. Like any habit, after getting used to it, it does not cost any significant additional time in daily work. Current Status and Planing of CWS warnings01 In the first round, we will fix all GCC warnings on Linux, after that the remaining warnings on Windows and Solaris. The initial goal was to use the default warning set, but the team decided that they want to start with “wall=true” and see if they can avoid all warnings on that level. If so, wall=tr will become the default later. It can happen that a module compiles fine without warnings, but delivers some header files which lead to warnings in other modules. To avoid this, we should test all delivered header files. This could be done with the old “testhxx”. If someone think it's reasonable to suppress a warning, this must be discussed with the team members. It should also be discussed in public, using the mailing list [EMAIL PROTECTED] Please start the subject with “compiler warnings:”. In external projects we only want to fix the header files. In case we want warnings to become errors later, to keep the code warning free (not decided yet), we must find a solution for this modules. We discussed that all modules should be build with exception handling turned on, because this avoids a lot of warnings in some modules. The team members should also have a look at the 64-bit CWS (ooo64bit02) to see what data sizes they already changed. When typecasting string length, please use XubStrLen instead of USHORT (old tools String), so it's easier to find them in case we change the old String to 32bit some time. This might also be a good idea for some other sizes, e.g. from tools/svtools containers. 2 FIRST WORK ON LOW-LEVEL MODULES I started builds for all Hamburg-relevant platforms (unxlngi6, unxlngi6.pro, unxsoli4.pro, unxsols4, unxsols4.pro, wnmtsci10, wntmsci10.pro). On unxlngi6.pro (the platform we want to focus on first), I now reached a point where others can probably pick up and start work on their part of the module tree. I did the following changes until now: - To improve consistency, and to ease further modifications, handling of C/C++ compiler warning switches in solenv has been changed as follows: - CFLAGSDFLTWARN has been replaced by two new variables CFLAGSWARNCC (C compiler) and CFLAGSWARNCXX (C++ compiler); if on some platform CFLAGSWARNCXX is not set, CFLAGSWARNCC is used instead; - CFLAGSWALL has been replaced by two new variables CFLAGSWALLCC (C compiler) and
Re: [dev] Warning-Free Code
On Mon, Aug 29, 2005 at 05:59:29PM +0200, Stephan Bergmann wrote: Hi all, Here at Sun Hamburg, we are eager to get OOo's C/C++ code base warning free. We plan to do this massive (in number of files affected) change for post--OOo-2.0. Since doing these changes needs close collaboration (think of headers from one module that only cause warnings once they are used in another module, for example), we initially plan to do this with a Hamburg-only group of engineers, working together on (a single physical copy of) CWS warnings01. If the work becomes too boring for us, we may start to ask for help from the community... ;) That's great, though seriously tedious work. I had thought about tackling the warnings incrementally by making one warning at a time into an error and working up to full warning coverage, but if there's the resources in Hamburg to do it in one big bang that's much better, I can sit on my ass instead :-) C. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning-Free Code
From: Stephan Bergmann [EMAIL PROTECTED] Date: Mon, 29 Aug 2005 17:59:29 +0200 Hi, Here at Sun Hamburg, we are eager to get OOo's C/C++ code base warning free. great. Can you do all of your work on 64bit operating systems? ;-) The team members should also have a look at the 64-bit CWS (ooo64bit02) to see what data sizes they already changed. Yes! -- Pavel Janík It's known to be very buggy (it comes directly from the hardware manufacturer). -- Hubert Mantel about some driver for eth. card - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [dev] Warning-Free Code
From: Stephan Bergmann [EMAIL PROTECTED] Date: Mon, 29 Aug 2005 17:59:29 +0200 free. We plan to do this massive (in number of files affected) change for post--OOo-2.0. I forgot to ask in first e-mail. Post-2.0. You think of 2.1 or 3.0? Or? -- Pavel Janík Let your compiler do the simple optimisations. Don't strain to re-use code; reorganise instead. -- The Elements of Programming Style (Kernighan Plaugher) - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
