Repository navigation
Static symbols considered harmful #269
Description
... by the GNU tools themselves, see e.g. http://stackoverflow.com/questions/21679940/include-static-symbols-in-gnu-ld-map-file , expected outcome: that's not possible.
So, instead of "static void foo()" we should do "STATIC void foo()", and then define STATIC up and down. Let me know if you agree and I can proceed with the changes.
Activity
jugglerchris commentedon Feb 10, 2014
Sorry if I've missed the context, but what's the actual problem with not including static symbols in object files?
dhylands commentedon Feb 10, 2014
The symbols are actually included in the object files, and the debugger has no problem finding them.
The only downside that I'm aware of is that the symbols don't show up in the .map file (which we don't currently generate).
dpgeorge commentedon Feb 10, 2014
But I like self-contained modules and not having to worry about names clashing with other modules. Eg, I can define the same function name in 2 separate files, have it static, and not worry about a name clash.
To prevent this, you need to name all functions in a file as though they are in a global namespace.
jugglerchris commentedon Feb 11, 2014
Making things static also helps the optimiser, as it can make more assumptions if it can see all users of a function (or data). eg leaving out unused ones to save space. :-)
pfalcon commentedon Feb 11, 2014
I like that too, but what to do if toolchain has gaps in overall support of them throughout the development process?
Unfortunately, situation is far from being able not to worry. For example, try to do "disass emit_pre" in gdb - it will show a random one, without telling which, and without giving a hint there're others.
So, just to clarify, I propose to change almost all static's to STATIC, and have that defined to "static" as before. For "analysis" builds, it will be defined to empty string. str_join dup case is easy to fix, emit_pre can stay as "static", though I'd go for prefixing it and doing
#define emit_pre <file>_emit_pre, to not have any "hidden" functions.dpgeorge commentedon Feb 11, 2014
As pointed out above by @jugglerchris, proper static is definitely needed for proper optimisation (given that the compiler is good enough; gcc is).
I also don't like macro creep: that everything starts to be written in a custom macro-language within C. For efficient embedded code, that's kind of a necessary evil. But still would like to keep things non-macro based if possible.
Anyway, we can go ahead with this change, using STATIC instead of static for functions.
What about static data?
pfalcon commentedon Feb 12, 2014
Not challenging that, but to keep up the conversation: indeed, static allows for limited (intra-module) optimizations, but if someone is after optimization, then the way to do it is whole-program optimization. Then surprisingly it's not important if symbols are marked as static or not (compiler treats all symbols as such).
Well, we're getting of the original topic, but why then we have stuff like MP_OBJ_NEW_SMALL_INT()? It makes eyes burn and breaks continuity in the API. I understand intention to remind that it's a macro and can have side effect issues due to double evaluation, but that function is part of API set, and should be named consistently, and any (if at all) side effect issues should be resolved separately. We may also want to switch to inline functions, but ironically, to get them working decently, we'll again need stuff like "INLINE".
The idea is that all global symbols should be "STATIC", exactly to allow locals to be freely "static" if needed (local statics are unlikely needed for language core, but who are we to disallow their usage in hw drivers, etc.?)
pfalcon commentedon Feb 12, 2014
Ok, done. Trivia:
dpgeorge commentedon Feb 12, 2014
Okay, very good.
I agree with you re
MP_OBJ_NEW_SMALL_INT. Making it an inline function would be the best solution. Failing that, I'd rather have it a call than a macro.pfalcon commentedon Feb 15, 2014
Ok, let's close this, I opened #293 for macro/inlines issues.