Author Topic: New compiler line type: cltInfo  (Read 33461 times)

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9496
Re: New compiler line type: cltInfo
« Reply #15 on: June 20, 2007, 06:00:55 pm »
Ok, I'll update it and pump up the version (again) to 0.0.3.
Be careful: killerbot did already some change in r 4115.

:P Just a last question. Should cltInfo be the second, or the last? Because in other files i assummed that cltWarning will always be greater than cltInfo.
I believe the ">=" stuff won't work and I guess it's really better to use a switch case construct... what do you think?

With regards, Morten.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ

Offline mandrav

  • Project Leader
  • Administrator
  • Lives here!
  • *****
  • Posts: 4291
    • Code::Blocks IDE
Re: New compiler line type: cltInfo
« Reply #16 on: June 20, 2007, 06:03:41 pm »
Initialize cltNormal to 0 too, so we have predefined numbers...

You mean it's initialized to ONE???  :shock:

No, I mean you don't really know what number the compiler will use for the first enumerator. It should be 0 but you don't know it for sure, and certainly not if you use a different-than-gcc compiler.
So, what I 'm saying is initialize the first enumerator to 0 and everything will be well defined.
Be patient!
This bug will be fixed soon...

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: New compiler line type: cltInfo
« Reply #17 on: June 20, 2007, 06:28:41 pm »
No, I mean you don't really know what number the compiler will use for the first enumerator. It should be 0 but you don't know it for sure, and certainly not if you use a different-than-gcc compiler.
Hmm actually that's in the C++ standard... but it certainly does not hurt to explicitely set it to 0. First, to account for broken compilers, and second for human readers who might possibly expect something else :)


I believe the ">=" stuff won't work and I guess it's really better to use a switch case construct... what do you think?
I use such constructs every day, they work nicely (as it happens, the new logging system distinguishes info from error like this too). enum values start at 0 and are incremented by 1 unless you tell the compiler something different. Thus, by properly ordering the enums (which is sensible, anyway), you can determine the "general nature" of a "thing" very easily and efficiently.

This btw. is one of the few things that are implemented really, really well in wxWidgets. On all stock items and all items created with defaults, you can tell from an item's (control, font, cursor, colour, language, encoding...) ID what it is.
Well, you should of course not do it for compatibility's sake, but the toolkit can tell what "unknown thing with ID foo" is, and that's pretty darn cool.
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5177
Re: New compiler line type: cltInfo
« Reply #18 on: June 20, 2007, 06:37:45 pm »
in the enum the first entry is 0, C/C++ does that, but we added it explicitly to make it more standing out ;-)

DO NOT change the order of the enum anymore, I fixed it. New enum values have to be added to the end, don't insert !!!!

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9496
Re: New compiler line type: cltInfo
« Reply #19 on: June 20, 2007, 06:43:03 pm »
I use such constructs every day, they work nicely (as it happens, the new logging system distinguishes info from error like this too).
Sure you can use it if it makes sense. But after the latest change the order of the items will be:
Code: [Select]
enum CompilerLineType
{
  cltNormal = 0,
  cltWarning,
  cltError,
  cltInfo
}
Tell me how you get the same functionality in those cases with >=???
With regards, Morten.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5177
Re: New compiler line type: cltInfo
« Reply #20 on: June 20, 2007, 06:57:15 pm »
please please, don't reorder. The int values are being exported, you can avoid to break stuff, why break it, because it would be nice to have in the enum first  info, then warning and then error.

The next time there's something else then info, warning, error. Then what ? Reorder and break again.
Be explicit in switch, case :  code is much easier to read and less error prone.

Offline rickg22

  • Lives here!
  • ****
  • Posts: 2283
Re: New compiler line type: cltInfo
« Reply #21 on: June 20, 2007, 08:22:35 pm »
OK Killerbot. I won't touch it. Just make sure you've replaced my >= with ifs/ cases. (You can do a revision graph to see which lines i had changed).

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5177
Re: New compiler line type: cltInfo
« Reply #22 on: June 20, 2007, 09:51:55 pm »
A little recap
Starting this analysis with the old code before the changes of today.
From the GUI where you can add the types you want and you can select a line type :
In a combo box : Type :
  Info
  Warning
  Error

so I guess the SaveRegexDetail : rs.lt = (CompilerLineType)wxComboBox:GetSelection() would return for the above entries
  0
  1
  2

Now when we look at the enum :
Code: [Select]
enum CompilerLineType
{
    cltNormal
    cltWarning,
    cltError
};

We see that info line types were mapped to cltNormal (0), and warning to cltWarning (1), and the error to cltError (2).

So all is well.

However if we want to treat the info separate from normal in the code, we should expand the enum. And because the number value of the enum is in the default.conf file for the compilers, we should expand it at the end. Not insertion, because that will cause a shift and things will be misinterpreted.

So we get :
Code: [Select]
enum CompilerLineType
{
    cltNormal
    cltWarning,
    cltError,
    cltInfo,
};

Now we need to adjust also the gui part. There we have to make sure that an info line (type) ends up being   a cltInfo.
However it used to map onto a cltNormal. So here we surely change things and some first side effects in certain configurations might become visible.

Question here is : do we also add Normal to the combo box (?), so 4 choices, reflecting the 4 values of the enum. And if we want to do it with a simple cast like the original code, we need something like this :

Code: [Select]
<content>
<item>Normal</item>
<item>Warning</item>
<item>Error</item>
<item>Info</item>

Let's do that for now. One might ask why add a regex if I want it to be just normal (normal things --> don't do anything specific in the code for those ...).
But on the other hand, we have a better match then.

Consequence : people already having "info entries" which were treated as "normal line types" will continue to see them in the GUI of the advanced settings as Normal and no longer as Info. So if you really want those to be treated as Info (whatever that means : see below) will have to change this through the gui.

So moving on to the next big question. How to deal with info's, now we conider them to be different from 'Normal'.
We have a and continue to have :
1) errors are shown in red and show up in the build messages
2) warnings are shown in blue and show up in the build messages
3) normal shown in black and don't show up in the build messages.

Now what is an info ?? For example in the Windriver compiler, an info is actually more of a warning. Maybe a less severe warning, but I have to say some of that compilers infos were pure warnings for other compilers, and that's the same opinion I had with those I cam across.

Todays changes had put the info in +- category 3) .
I feel they should be treated as warnings. Code should be error free, warning free, and info free ;-)

But let's keep the changes made today for mow. Feel free to tell your opinion bout this.

So we should change a ew more  lines of code (compiler.gcc) :
 - log to the build messages as long as the type isn't normal  (AddOutputLine)
 - but show it in the build messages in the system color for text (+- black) (like normal line type)  (LogWarningOrError)
 - in the log file (html) choose a correct colored font for it (LogMessage) ==> here the code was +- okin the past but certainly not correct, because a font tag was opened for error/warning or when forceErrorColour BUT it was only closed when it was not of type 'normal'. Obvious that when forceErrorColour was true and it was of type normal, that the tag would never be closed.

The line should have said, and should now, since info is treated colorwise as normal : close the font tag if 'error or warning or forcedErrorColour'.


So i will commit because of this story :
- xrc with the resynched entries
- compilergcc.cpp for that font tag closing

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9496
Re: New compiler line type: cltInfo
« Reply #23 on: June 20, 2007, 10:15:34 pm »
I feel they should be treated as warnings. Code should be error free, warning free, and info free ;-) [...] Feel free to tell your opinion bout this.
As you have stated already, just to underline:
Normal = default message usually of no great importance at all -> build log
Info = message that better be put into build msgs because it's not a default message
Warning = message that better has to be put to the build msgs because something is wrong
Error = message that definitely has to be put to the build msgs because something went wrong
So I feel the Info category fits well in what the compiler tries to tell. But I agree totally with you: Compilation should also be "Info" free.

Coming to the enum / GUI value: I personally would like to see the ordering as Rick did it in the first place in code and the the same for the GUI. In daily live most of the people will click "OK to change settings" if the appropriate C::B warning has appeared (at least that's what happened at my work... ;-)). Having the enum sorted by importance has several pros imho. So here are my two cents: Ignore backward compatibility this time. :P Again: That's only my opinion.

With regards, Morten.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ