GCC attributes

[Der Meister]

From revision 1502 on a file named 'src/sdk/gcc-attribs.h' entered the project. At first this seems to be no problem but in revision 1507 there were massive changes to this file. One of the changes was adding the makro __const__. This breaks (at least vor me) the codestats plugin (and possible more - it could even break the whole project. Seems as this didn't happen yet, I fear it could do it when nobody would expect it.). The problem is: The file '/usr/include/bits/mathcalls.h' contains lines like the following:

Code

__MATHCALLX (ceil,, (_Mdouble_ __x), (__const__));

Now guess what happens  :?
GCC throws lots of weired error messages. Error messages that seem to have no sense at all.
That is the best example why praeprocessor macros are evil. It took me two hours to find the reason for that problem.

Anyway, why is Code::Blocks even using gcc extensions? Using compiler specific things (especially compiler specific extensions) is a very bad practice (at least in my opinion). Is there any reason why Code::Blocks now uses them? It should be a very very good one - or I'll be rather disappointed. 

[rickg22]

Let's see what WebSVN says...

Rev: 1507
Path: /trunk/src/sdk/gcc-attribs.h
Author: thomasdenk
Age: 2 days
Log message:
Changed gcc attribute macros to lowercase for better readability.
Changed likely() and unlikely() so they work with arbitrary values.
Added some more attribute macros.
Added attributes to the block allocator and to code completion.
Marked Manager::isappShutingDown() as deprecated.

Edit: I mailed him, I'm sure it'll be fixed in the next revision.

[takeshimiya]

Don't we love SVN? 

Yes.
And TortoiseSVN have a function just for this.

Just go to /src/sdk, and right-click on gcc-attribs.h ->TortoiseSVN->Blame.
And then, it will tell you the person to blame.

[thomas]

Well, apologies for your inconvenience. What can I say... the macro redefines a keyword, which is of course illegal (but which normally works nevertheless). I'll change the macros so they use single underscores instead of double underscores, this should fix your problem.
And yes, you are right, this is a good example why macros are evil.

Anyway, why is Code::Blocks even using gcc extensions? Using compiler specific things (especially compiler specific extensions) is a very bad practice (at least in my opinion). Is there any reason why Code::Blocks now uses them? It should be a very very good one - or I'll be rather disappointed. 

This is easily explained. I introduced gcc extensions because they are there, and because you can use them. It is bad practice to use them if you do not take provisions that you can still use other compilers, but these were taken. The macros evaluate to a gcc attribute definition when compiled with gcc, and to an empty string in all other cases. I do not see that as bad practice, I see it as making use of features which are useful. For example, if(unlikely(ptr == 0)) gives valuable hints to the compiler - the branch for the null pointer is almost never taken, and gcc is able to optimize the code accordingly. On compilers other than gcc, unlikely(x) evaluates to (x), so you don't really give away anything.
Another good example is the function IsAppShutingDown. This function is misspelled, but for backwards compatibility, it is still in the API. Now, thanks to attributes, that function is marked with the deprecated attribute. Not only can you see it in the header, but if you actually use that function, gcc will tell you that it is deprecated and you should not be using it. I see that as an improvement.

The problem you encountered is not due to using attributes, but because I chose a bad macro name. Prior to 1507, the macros were in capitals, but PURE and DEPRECATED are so darn ugly, hence I changed them. __const__ and __inline__ were indeed bad choices, granted.

[Urxae]

[The risk of taking a long time to post: you get beaten to it :?]

From revision 1502 on a file named 'src/sdk/gcc-attribs.h' entered the project. At first this seems to be no problem but in revision 1507 there were massive changes to this file. One of the changes was adding the makro __const__. This breaks (at least vor me) the codestats plugin (and possible more - it could even break the whole project. Seems as this didn't happen yet, I fear it could do it when nobody would expect it.). The problem is: The file '/usr/include/bits/mathcalls.h' contains lines like the following:

Code

__MATHCALLX (ceil,, (_Mdouble_ __x), (__const__));

Now guess what happens  :?
GCC throws lots of weired error messages. Error messages that seem to have no sense at all.
That is the best example why praeprocessor macros are evil. It took me two hours to find the reason for that problem.

Well kids, remember this: don't use names that are reserved for the (C++) implementation, such as those containing a double-underscore, except as defined in the documentation of that same implementation .

Anyway, why is Code::Blocks even using gcc extensions? Using compiler specific things (especially compiler specific extensions) is a very bad practice (at least in my opinion). Is there any reason why Code::Blocks now uses them? It should be a very very good one - or I'll be rather disappointed. 

First thing, notice that the entire contents of the file (except include guards) are contained in an #if __GNUC__ >= 3 / #else / #end block, so the use of the macros defined in it shouldn't cause any problems on compilers other than gcc. (at least not on ones that don't define __GNUC__, which would be rather improper to do if you're not 100% source-compatible with gcc)

The reason to use such attributes is generally that they provide the compiler with extra information about your program that it can use to optimize it better. Since they are compiler specific extensions however, syntax and availability differs between compilers and thus packaging such attributes in macros is the usual solution. MSVC has extensions for at least some of those used in the file using the __declspec syntax, for example.
The reason to expand to nothing if not compiled on gcc instead of using those attributes is likely that C::B currently (AFAIK) doesn't compile on any other compilers anyway. If it did, they could be defined to something else in an #elif clause for other compilers.

[rickg22]

It's interesting to see how different people think differently (doh). Thomas has an obsession with optimizing the implementations (i.e. blockallocator), while i have an obsession with optimizing the algorithms. I think we make a good combination

[takeshimiya]

Urxae is right, never define something starting with an _ (underscore) or __ (double underscore), because those are reserved for usage by the implemetator of the standard library, so it's probable to have name clashes.

As Stroustrup says also, having macros in UPPERCASE helps. Appart, as they are a little more hard to read, you try to not use them whenever possible.

Also, wxWidgets have interesting macros in defs.h, and IMHO you should use them whenever possible (don't reinvent the well like:

/*  Macro to issue warning when using deprecated functions with gcc3 or MSVC7: */
#if wxCHECK_GCC_VERSION(3, 1)
    #define wxDEPRECATED(x) x __attribute__ ((deprecated))
#elif defined(__VISUALC__) && (__VISUALC__ >= 1300)
    #define wxDEPRECATED(x) __declspec(deprecated) x
#else
    #define wxDEPRECATED(x) x
#endif

So, if you are about to introduce a new macro that it's not defined in wxWidgets, a good name would be something as cbUNLIKELY.

[thomas]

Thanks, I knew about these, but I don't like their syntax (putting everything into a macro argument), and they only define a few attributes. For example, you cannot write int someFunction() wxDEPRECATED; which is a lot better to read.

Also, wxWidgets is notorious about defining really sick macros (#define new) and using attributes incorrectly. Have you ever wondered why wxWidgets produces about 100.000 warnings during a normal compile? Most of them are related to incorrectly used attributes.

[takeshimiya]

Also, wxWidgets is notorious about defining really sick macros (#define new) and using attributes incorrectly.

First, it can be disabled with wxUSE_GLOBAL_MEMORY_OPERATORS.
Second, they must have a reason for doing that.
Third, if you are complaining about that, why don't send a feature request or patch?
Four, if you know things that you think are wrong in wxWidgets itself, and can do nothing about it, could you make a separate thread (or @WiKi) indicating things to "avoid" in wxWidgets, so other users can benefit from that.

Now back on topic:
Saying that wxDEPRECATED(x) is harder to read is an opinion. In my view, is better because: it's capitalized, and it notes that the entire macro is applied.
However all of thatisn't that important compared to this:

While your someFunction() cbDEPRECATED works on GCC, it doesn't work on Visual C, because the declaration is inverted cbDEPRECATED someFunction().

So, putting everything into a macro argument like wxDEPRECATED(x) is the only solution.
You said "I knew about these", but it seems you didn't read what the wx #define does and how it does.

I hope you'll understand.

[Urxae]

Urxae is right, never define something starting with an _ (underscore) or __ (double underscore), because those are reserved for usage by the implemetator of the standard library, so it's probable to have name clashes.

While true, the complete rules are a bit more complex:

In C, the implementation reserves

• any global-scope name beginning with _
• any name beginning with _ followed by an upper-case letter
• any name beginning with __

C++ additionally reserves any name containing __ anywhere in it, not just at the beginning.

Also, "the implementation" covers both the standard library and the compiler itself.

[Urxae]

Now back on topic:
Saying that wxDEPRECATED(x) is harder to read is an opinion. In my view, is better because: it's capitalized, and it notes that the entire macro is applied.

It's an opinion I agree with though. Wrapping the entire declaration in a macro makes it harder to see what is being declared, or that it is in fact a declaration at all. Not to mention what happens when you want to apply multiple attributes...

However all of thatisn't that important compared to this:

While your someFunction() cbDEPRECATED works on GCC, it doesn't work on Visual C, because the declaration is inverted cbDEPRECATED someFunction().

So, putting everything into a macro argument like wxDEPRECATED(x) is the only solution.

It's not. At least not if MSVC and GCC are your targets, I don't know how other compilers handle attributes.
You forgot an option: just use cbDEPRECATED someFunction(). My g++ accepts attributes in that location:

Code

D:\Temp>cat test.cpp
void __attribute((deprecated)) f() {}

int main()
{
    f();
}

D:\Temp>g++ test.cpp
test.cpp: In function `int main()':
test.cpp:5: warning: `f' is deprecated (declared at test.cpp:1)
test.cpp:5: warning: `f' is deprecated (declared at test.cpp:1)

Anyone want to bet why this works, by the way?
My money is in order to support exactly this situation: using macros to apply attributes without having to wrap the entire declaration in them .

[takeshimiya]

It's ok if GCC supports that then. But are you sure that works on any gcc version?

And you'll certainly will want to hope that no other compiler handles that in a different way.

With the other way however, you don't have to worry about future/other compilers.

[thomas]

It's ok if GCC supports that then. But are you sure that works on any gcc version?

And you'll certainly will want to hope that no other compiler handles that in a different way.

With the other way however, you don't have to worry about future/other compilers.

No, some of these attributes only work with gcc version >= 3.

You saw the #if, didn't you

[Urxae]

Replaced double-underscore macros with triple-underscore due to problems encountered with codestats plugin.

While this may solve the immediate problem, it doesn't fix the underlying cause. You're still using names that are reserved to the compiler and standard library. Please reread my post above:

In C, the implementation reserves

• any global-scope name beginning with _
• any name beginning with _ followed by an upper-case letter
• any name beginning with __

C++ additionally reserves any name containing __ anywhere in it, not just at the beginning.

Triple underscores still contain double underscores, plus they begins with a double underscore, plus they're in the global scope (macros are in every scope ) so even that clause is violated here. In fact, the only clause this doesn't violate is the one about underscore-upper case letter, simply by virtue of not containing any uppercase letters.

How about something like cb_inline_/cb_inline? Or just going back to upper case: cb_INLINE_/cb_INLINE? They're macros, general convention is that they contain a lot of (if not only) uppercase symbols anyway...

Oh, and looking over that patch I noticed something else: The deprecated attribute on Manager::isappShutingDown() is at the end of the line. As mentioned in earlier posts it'll need to be more to the front for MSVC, and GCC accepts it there too. Might be a good idea to move it, though by the time MSVC is supported that method may have been removed completely already .

[takeshimiya]

I vote for cbEVILMACRO convention.

[sethjackson]

I vote for cbEVILMACRO convention.

I'll second. :lol:

[rickg22]

I third. I believe that's the way it should be.

[thomas]

Lest someone pronouces fo(u)rth, I removed gcc attributes.

[Urxae]

Lest someone pronouces fo(u)rth, I removed gcc attributes.

Nobody said they had to go entirely, just that the following spelling was preferred to the one with all the underscores:

Code

// Attributes and builtins used when compiling Code::Blocks with gcc
// No effect (short of being documentary) with other compilers
//
// $Id$

#ifndef GCC_ATTRIBS
#define GCC_ATTRIBS

#if __GNUC__ >= 3   // ----------------------------------

#define likely(x)               __builtin_expect(!!(x),1)
#define unlikely(x)             __builtin_expect(!!(x),0)
#define compiler_constant(x)    __builtin_constant_p(x)
#define cpu_prefetch(x)         __builtin_prefetch(x)

#define cbINLINE                __attribute__ ((always_inline))
#define cbNOINLINE              __attribute__ ((noinline))
#define cbNOTHROW               __attribute__ ((nothrow))
#define cbDEPRECATED            __attribute__ ((deprecated))
#define cbCONST                 __attribute__ ((const))
#define cbPURE                  __attribute__ ((pure))
#define cbCHECKRESULT           __attribute__ ((warn_unused_result))
#define cbMALLOC                __attribute__ ((malloc))

#ifdef __WIN32__
  #define cbFASTCALL            __attribute__ ((fastcall))
  #define cbFASTESTCALL         __attribute__ ((regparm(3)))
#else
  #define cbFASTCALL
  #define cbFASTCALL
#endif

#else               // ----------------------------------

#define likely(x)               (x)
#define unlikely(x)             (x)
#define compiler_constant(x)    false
#define cpu_prefetch(x)

#define cbINLINE                inline
#define cbNOINLINE
#define cbNOTHROW
#define cbDEPRECATED
#define cbCONST
#define cbPURE
#define cbCHECKRESULT
#define cbMALLOC

#define cbFASTCALL
#define cbFASTCALL


#endif              // ----------------------------------


#endif

[killerbot]

I just looked at 1 file that Thomas changed, tokenizer.h
I see some methods being defined within the class body, add also the keyword inline specified.

The keyword inline is not that interesting, since it is just a hint to the compiler !!!

Secondly, methods being implemented within the class definition/body are , according to the standard, to be inlined !!!


Lieven

[thomas]

The keyword inline is not that interesting, since it is just a hint to the compiler !!!
Secondly, methods being implemented within the class definition/body are , according to the standard, to be inlined !!!

So what do you want to tell me? Functions defined inside the class scope are only inlined in optimized build.
Code::Blocks is currently being built and distributed non-optimized. In the special case of the code completion parser, this makes a difference of nearly 100% in execution time. This was one reason for using function attributes (in particular always_inline).

The inline keyword gives an additional hint to the compiler that I wish these functions to be inlined, optimized build or not. Maybe the compiler will ignore the hint, maybe not. There is not much harm in using it. However, if that makes you feel that I don't know C++, then I'll remove it.

[killerbot]

Thomas,

Methods implemented in the class body are ALWAYS inlined  (I think this is guranteed by the standard) !!!

class MyClass
{
  void MyFunction1() const {return something;}  // <<---- always inlined
  inline void MyFunction2(); //<<-- implemented somewhere else (90% chance this inline suggestion will be ignored)
}


I don't doubt your C++ knowledge !! That sounded very negative, I hope it did not give you this impression.

[thomas]

Methods implemented in the class body are ALWAYS inlined  (I think this is guranteed by the standard) !!!

[...] Otherwise, when you specify -O, member functions defined inside class scope are compiled inline by default

[me22]

Methods implemented in the class body are ALWAYS inlined  (I think this is guranteed by the standard) !!!

class MyClass
{
  void MyFunction1() const {return something;}  // <<---- always inlined
  inline void MyFunction2(); //<<-- implemented somewhere else (90% chance this inline suggestion will be ignored)
}

Correction: functions defined inside a class definition are inline-qualified.

Note, however, that that does NOT mean "always inlined."  inline is a request, not an order.  Compilers are permitted to completely ignore the request, and many do.

GOTW #33: Inline covers this nicely:

Inline Difficulty: 4 / 10
Contrary to popular opinion, the keyword inline is not some sort of magic bullet. It is, however, a useful tool when employed properly. The question is, When should you use it?

It's also mentioned in GOTW #27:

This one is a matter of judgment. In short, prefer to write all functions out-of-line by default, and then selectively inline individual functions as necessary only after you know that the performance gain from inlining is actually needed.

If you inline the function, the positive side is that you avoid the overhead of the extra function call to f.

The negative side is that inlining f exposes f's implementation and make client code depend on it, so that if f changes all client code must recompile. Worse, client code now also needs at least the prototype for function g(), which is a bit of a shame since client code never actually calls g directly and probably never needed g's prototype before (at least, not as far as we can tell from our example). And if g() itself were changed to take other parameters of still other types, client code would now depend on those classes' declarations, too.

Both inlining or not inlining can be valid choices. It's a judgment call where the benefits and drawbacks depend on what you know about how (and how widely) f is used today, and how (and how often) it's likely to change in the future.

From the GotW coding standards:
    - prefer passing parameters of class type by const& rather than passing them by value
    - avoid inlining functions until profiling tells you it's justified (programmers are notoriously bad at guessing which parts of their code are performance bottlenecks)

( Also, void functions can't return something :lol: )

[killerbot]

Yes, me22 I have read this also in the bookd of Herb Sutter, and those of Scott Meyers.
Last month I was in a workshop of Scott Meyers about the preformance of STL, and using it the smart way. There was a topic about sorting and using functors, and if I recall correctly he mentioned that functors are better then regular functions, since many functors are small classes or structs, where the operator() is implemented directly in the class/struct defintion and therefor is inlined (I think he said the standard guarantees that). I will browse through the printed slides to see if I can find it black on white.

If my statement from above is correct, then maybe we might have a compiler behaving differently. I'll look for some more documentation on the subject.

kind regards,
Lieven

[killerbot]

From the slides of Scott Meyers workshop :

class Widget
{
public:
  int weight() const;
};


std::vector<Widget> vw;
std::sort(vw.begin(), vw.end(), );

What's best way for the sorting criterion ?
  - a function ?
  - function object ?

Function object might be like this :
struct WidgetLess : public std::binary_function<Widget, Widget, bool>
{
  bool operator()(const Widget& lhs, const Widget& rhs) const
 {return lhs.weight() < rhs.weight();}
}


stuff mentioned about the function object :
 * it's a class with an (implicitly) inline operator() function
 * compilers routinely inline calls to such functions

So does that first note state that it is guaranteed to be inlined ?? I am not sure (anymore?) ?
I am still convinced that if also in this case it is still a suggestion, it is as strong as specifying inline, or they are just the same.
In the c++ conference last month in Vegas, several speakers have 'said' that implementation in the class body == inline.

Though I'dd like to see this in black on white in printing, will browse through my books some evening.

[killerbot]

Scott Meyer's : Effective STL (item 46) : page 202
If a function object's operator() function has been declared inline (either explicitly via 'inline' or implicitly by defining it in its class definition), ....