On Tuesday 06 January 2015 23:25:39 Thomas Lübking wrote: > On Dienstag, 6. Januar 2015 18:15:02 CEST, Jan Kundrát wrote: > > - Report all reasonable warnings. > > +1 (for sure) >
+1 > > Warnings are IMHO quite useful because they can hint about > > possible problems in the code. > > If there's a known comflict (where we cannot avoid a warning > but know it's ok) and this gets too annoying, we can make use > of > http://gcc.gnu.org/onlinedocs/gcc-4.3.3/gcc/Diagnostic-Pragmas > .html > > > - Make an optimized build with debugging symbols present. > > +1 +1 > The compile time overhead of -O2 is neglectable compared to > the runtime gains. > "Usual" debugging is possible and *if* one runs into a > situation where one needs extended debug features, one likely > wants to gdwarf/2 as well. > > -O0 should imo not be default (ie. used by "ordinary" git > testers) > +1 > Occasional -O0 builds on all compilers (to ensure > buildability) could maybe done by the CI? > Automated builds without optimization could find another compile problems in future... > > The problems with these criteria are that some compilers > > produce a ton of warnings which are of boguous quality (at > > least some version of MSVC per users' reports). > > Do you have such warning output, resp. could somebody provide > it? MSVC used to be horribly "relaxed" and compiled all sorts > of ... junk (eg. case insensitive symbols and keywords), so > if it warns, this may due to reliance on gcc features or just > a problematic system header being included. > > > I haven't seen a MSVC output with -Wall yet > > Ah. ;-) > Can you recall the reporter and ask for such output? > > > My opinion is that the warnings should > > be present on unknown compilers, though, so my suggestion > > for a policy is to "show warnings unless known to be > > unbearable"). > > +1 > and i'd preferably enable them for MSVC (and sort out and > eliminate the cause instead - if possible) > Problem is that -Wall (or -W) is not standard option. Other systems (like Windows) are traditionally using slash instead dash when parsing options. Could be there some problem? > > Re optimizations, I currently don't see a use case for > > disabling them. Am I missing something? > > -O3 can increase the size and cause "weird" stack traces. > -O2 can make stepping through less reliable and provides less > value inspection > But, as mentioned, -O0 -gdwarf-2 (or -ggdb) does not seem > reasonable for the "ordinary" git user and if you feel like > you need it, you can enable it any time for that purpose. > -O2 can drop local variables from debug symbols (or inline lot of functions) and sometimes when debugging it is better to compile C++ application without optimizations. > > Re debug symbols > > Should stay by default. > +1 > Users which compile directly from git can probably be expected > to "strip" themselves and will usually also be able to obtain > backtraces as coredump or gdb on their own (or w/ a little > advice) > > It's the distros task to choose whether debug symbols make > sense for their audience. > This includes gentoo or AUR (which can adjust the flags or > install process via PKGBUILD or the use flags etc.) > > > I'm unsure about the asserts. Do we want them on in release > > code? > > "Depends" > (Q_ASSERT is btw. noop in Release mode builds) > > This imo depends on how asserts are used. > For a code-by-contract approach, asserts can be a horrible > overhead (several check in hot loops...) and also cause > useless aborts (what makes users unhappy) > > But in case of > > - Foo *foo = static_cast<Foo*>(bar); > + Foo *foo = dynamic_cast<Foo*>(bar); > + Q_ASSERT(foo) > > the assert is the neglectable overhead (for internal base > classes, > > Also one should oc. seek to avoid slow calls in asserts. > - Q_ASSERT(slowCalculation()) > int i = slowCalculation(); > + Q_ASSERT(i) > > Cheers, > Thomas Another question is: What do you mean by "default" above? If you say that optimization should be enabled by default, does it mean -O2 is explicitly added to CXXFLAGS? Or by default is set CMAKE_BUILD_TYPE to RelWithDebInfo and optimization flags (depending on target compiler) are added to CXXFLAGS by cmake? (same question is for warnings and for debug symbols) I'm for setting default CMAKE_BUILD_TYPE to RelWithDebInfo and not for modifying CXXFLAGS. Reason: If for some reason I want to disable optimization and enable debug symbols in *cmake* project I would use -DCMAKE_BUILD_TYPE=Debug which do that. Overwriting optimization CXXFLAGS for all CMAKE_BUILD_TYPE can mess CXXFLAGS added by user and by cmake. Jan is against this solution because he thinks that somebody can compile trojita accidentally without optimizations (and trojita will be slow). But if somebody set CMAKE_BUILD_TYPE to some type without optimization then I think he would except that cmake will compile trojita without optimization... -- Pali Rohár pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
