PATCH: CB, compiler flags (new ones and improved handling) and possible crash

[White-Tiger]

Simply added more compiler options that I use frequently and defining them just for me is odd
I'm sure others need them as well, and this "custom flags" xml file was often overwritten in the past by Code::Blocks, so I prefer to not use it and use Code::Blocks default instead. (this also allows me to get new flags automatically, as using my custom file won't get new flags added automatically).

This patch also allows the use of spaces in compiler flag names as shown in screenshot (and this improves usability a lot, why remove spaces anyway? Ok I know... it was easier to do so xD)


patched:

• src/plugins/compilergcc/compilerflagdlg.cpp (handling, allow spaces in compiler flag names, also faster )
• src/plugins/compilergcc/compileroptionsdlg.cpp (handling, allow spaces in compiler flag names, also fixed option saving)
• src/sdk/compileroptions.cpp (fixes bug that enabled wrong options with just linker flags. Just don't search for empty options)
• src/plugins/compilergcc/resources/compilers/options_clang.xml (32 and 64 bit target, PIC, C99)
• src/plugins/compilergcc/resources/compilers/options_common_optimization.xml (added Link-Time-Optimization to flags)
• src/plugins/compilergcc/resources/compilers/options_common_warnings.xml (C99 for GCC)
• src/plugins/compilergcc/resources/compilers/options_gcc.xml (optimization for debug build -Og, 32 and 64 bit target, PIC, static linking)
• src/plugins/codecompletion/classbrowserbuilderthread.cpp (possible crash)
• src/src/debuggersettingscommonpanel.cpp (simple spelling fix)

PS: yes, one might think just add -flto as single option and add to those -O# flags also the corresponding linker flag even when not used.
But LTO actually works independently from compiler optimization... according to GCCs docs, it doesn't really matter if you're using -O# on compile time as the linker optimizes to it's given level anyway.
Since you can use LTO with and without compiler optimization, I let the developer decide if he wants to combine them or not. See http://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-flto-934

[oBFusCATed]

• src/plugins/codecompletion/classbrowserbuilderthread.cpp (possible crash)
• src/src/debuggersettingscommonpanel.cpp (simple spelling fix)

I'll commit these two in a moment.

• src/sdk/compileroptions.cpp (fixes bug that enabled wrong options with just linker flags. Just don't search for empty options)

The code style used to fix this looks bizarre!

About the LTO stuff: For me we need a better solution. There is no need to specify the -O flag twice. It should automatically replicate it in the linker settings when -flto is specified. And the options look weird or hacky in your screenshot. If I haven't read your description I wouldn't be able to guess that the O flags are linked to the LTO flag.

About the other changes: Can't comment much, probably we should include them, because some of the flags are often requested.

p.s. thank you for making me read the LTO's help agian, now my LTO builds are faster (as expected) than non LTO builds. In the past they have been twice as slow. It seems that the -O flag in the linker settings is really important:)

[White-Tiger]

[...]

• src/sdk/compileroptions.cpp (fixes bug that enabled wrong options with just linker flags. Just don't search for empty options)

The code style used to fix this looks bizarre!
[...]

... that was a hot fix actually xD
I use this style from time to time.. but wouldn't have recommended it this way for the patch... it's missing at least a line feed
But this change is necessary... until the @note is solved to not add empty options... (linker only options for example conflict a bit... took me a while to find the cause. Still not sure if it's fixed once and for all... I don't remember a recent issue though)

The options parser should be improved anyway... working with MSVC showed me again some of it's flaws.. such as that the order in which options appear are of importance... "/Zi /debug" is seen by Code::Blocks, but not "/debug /Zi" or even "/Zi", "/debug" (single, independent options not set as a "single option" even though their order is correct)

The parser also requires a new attribute... "dependencies"... enabling this options does not only disable X but also enables Y (disable kind of works, but enable is simply missing)

[...]
About the LTO stuff: For me we need a better solution. There is no need to specify the -O flag twice. It should automatically replicate it in the linker settings when -flto is specified. [...] If I haven't read your description I wouldn't be able to guess that the O flags are linked to the LTO flag.
[...]

you're basically right... but hard to implement and it's also hard to still allow freedom. (like I say on next quote)
For me it's actually easier to find LTO settings... would prefer such "grouping" more often. It's hard to follow pure lines of text without anything you can use to determine your current position^^ (or in case of optimizations, group them to easily see what's what.)
And I know that you can select categories... but that takes longer then to read the list at once.. (if you're enabling settings from different categories)

[...]
p.s. thank you for making me read the LTO's help agian, now my LTO builds are faster (as expected) than non LTO builds. In the past they have been twice as slow. It seems that the -O flag in the linker settings is really important:)

Not just "really important", they are all that counts -O on compile time is (currently) useless... I guess it might be even better to not use it there as I guess it'll speed up the compile time.. what's the reason to optimize first, when that isn't used anyway?
But I'm not totally sure and don't know of reasons why someone might need also to optimize on compile time as well, so I'm allowing both.

[oBFusCATed]

But I'm not totally sure and don't know of reasons why someone might need also to optimize on compile time as well, so I'm allowing both.

If you're linking without LTO, then you need optimized object files.

But then putting linker options in the compiler options is just wrong.
A better patch would be adding the same/similar UI to the linker settings.
So you have some of the common options there, too.

But generally the UI you've done for LTO's O flags require the use of another control something like wxpropgrid or even something more fancy.
In wxpropgrid one can have categories, setup trees, use comboboxes, it is a lot more powerful.

[White-Tiger]

[...]
But then putting linker options in the compiler options is just wrong.
[...]

that's how Code::Blocks does it eg. "-s" (strip) is linker only That's not the only option... you need 1 dialog for both... that's the right way actually. "-flto" for example needs to be passed on compile and link time.
So should the user search for where to setup what he needs? Search compiler options just to find out it's a linker option?
It's good to know how options work, but Code::Blocks should still be easy to use without requiring high knowledge.

About wxpropgrid ... does it allow combo boxes as well? UI would be cleaner if it's like:
[ x ] LTO (fully optimize)
where (xxxxx) is a drop down.
So it's only 1 line and allows to select which option to use.

Anyway.. I'm happy with the way it is now, and I'm not gonna changing it

[oBFusCATed]

Anyway.. I'm happy with the way it is now, and I'm not gonna changing it

BTW: Statements like this one lead to patches not been accepted 

@Alpha: Is it currently possible to make the -flto flag to force the -Ox flags to be passed to linker instead of the compiler or both?

[Alpha]

@Alpha: Is it currently possible to make the -flto flag to force the -Ox flags to be passed to linker instead of the compiler or both?

This cannot be done with the current implementation.  Do you have any recommendations of how such an option could be specified in the XML (or elsewhere)?

@White-Tiger: It appears you did some interesting things here, though I do not recall if I had seen it before.  ... Patches in a patch tracker are easier to not accidentally lose.

@devs: On SF, can we set up sub-pages for the tickets so actual patches do not get mixed with bug reports?

[oBFusCATed]

This cannot be done with the current implementation.  Do you have any recommendations of how such an option could be specified in the XML (or elsewhere)?

Probably we can have something like a group that marks optimization flags and the -flto flag has attribute that says that groupX should be passed to the compiler.

The interesting question here is: does only -Ox flags need to be passed to the linker or do we have to pass any other code generation flags?

[darmar]

I came to this topic when I tried to understand, what compiler options "   [-O1]   ", "   [-O2]   ", "    [-O3]   "  are for.

I think, that the current implementation is unclear for the users (and me). I would suggest to add some text to those options. Something like:
name = "Send -O1 option to the linker".
With such description user can better understand the meaning of these options.

[White-Tiger]

yeah.. it's a bit broken right now... Code::Blocks sorts all flags alphabetical now... that's why they're totally out of place.
It's probably a good idea to either find a proper way to handle LTO, or add them like this:

• Link-Time-Optimization (aggressive)[-flto -O3]
• Link-Time-Optimization (medium)[-flto -O2]
• Link-Time-Optimization (simple)[-flto -O1]

So that removes the single option (-flto) since it's always included (which makes kinda sense actually)
But that would brake my local build... since I've got this option in addition:

Code

Link-Time-Optimization (7 threads) [-flto=7]

Unless it's ok to have this on the command line: -flto -O1 -flto=7

P.S. I'm not fully sure about the naming.. but maybe we should find a short way to nickname O1 to O3... and O1 is generally safe to apply, while O2 and O3 can cause programs to crash (if their source is buggy etc.)

[oBFusCATed]

The original hack is to blame. The flag system must be improved to support drop downs/comboboxes. We're using wxpropgrid control which supports this, so it shouldn't be that hard to implement.

White-Tiger: Why your build will be broken? I didn't understand your comment.

[White-Tiger]

The issue with LTO is that it has at least 2 "independent" flags to configure...
The optimization level from 1 to 3 and the number of threads.

Though we might also just drop the optimization level from the linking step... (at least in the meantime to not confuse users)
“If you do not specify an optimization level option -O at link time, then GCC uses the highest optimization level used when compiling the object files.”

[oBFusCATed]

This is something new. Older GCCs (4.7, 4. used the lowest/no optimisation level.

I don't know why we have these options in the first place. They are in the wrong place and they are only for advanced users.

[White-Tiger]

Actually.. they're not "in the wrong place"...
You might be somewhat right to say they're linker options. But moving them outside the compiler dialog is still wrong.
1) the linker is part of the compilation process and part of the compiler
2) LTO options need to be send to BOTH the linker, and compiler. Except for the optimization options.. but having "LTO" on the compiler window, and LTO's optimization options on the linker window, would only confuse users... not to mention that people likely look for optimization options in the compiler tab.
3) a lot of other options are actually linker options already.. or at least also set options for the linker (which is still part of the compilation process. No one says they're linking a program... they compile it from source.)

P.S. the options went in because my entire patch got applied... and they used to work that time
It's bad to deny a feature just because it isn't "perfect" yet, better be able to make use of it than not at all.
P.P.S. I'm not really agreeing that it's for advanced users only... LTO got a lot of benefits and should be "default" for optimized builds. You wouldn't say that non-LTO -O is for advanced users only, would you?

[darmar]

Though we might also just drop the optimization level from the linking step... (at least in the meantime to not confuse users)
“If you do not specify an optimization level option -O at link time, then GCC uses the highest optimization level used when compiling the object files.”

I think, removing "-O1" etc. options is the reasonable solution. Options like "-flto=7" advanced users can add in the "Other linker options" field.