Allegro.cc - Online Community

Allegro.cc Forums » Programming Questions » to static or not to static?

This thread is locked; no one can reply to it. rss feed Print
to static or not to static?
Mark Oates
Member #1,146
March 2001
avatar

This is in reference to the static keyword.

I get a lot of "defined but not used" warnings like:

warning: 'void invert(ALLEGRO_BITMAP*)' defined but not used

Each of these are static inline functions that are in a universal header file, with the intention that whenever I may need to use the function it will be included, statically, inside that resulting object file. These functions are somewhat of a particular breed, akin to everyday functions like min/max, and don't necessarily fit into a specific category. I initially thought to go the static route because they're used so frequently I guess it would be better to get them "closer to the metal."

I don't know if my thinking is correct in this regard (looking for advice in that area.)

In order to alleviate the warnings, I'm trying to decide if I should:

  1. just jump the shark and make them regular functions that exist in a single object file, or

  2. silence the warnings with some preprocessor type code (if that can be done?) I don't want to silence anything via the compiler, at all.

  3. something else.

What should I do?

=-=-=-=-

There are other static inline functions elsewhere, but this is the file with the majority of offenders:

#SelectExpand
1#ifndef __USEFUL_HEADER 2#define __USEFUL_HEADER 3 4 5 6 7#include <sstream> 8#include <string> 9 10#include <allegro5/allegro.h> 11#include <allegro5/allegro_color.h> 12#include <allegro5/allegro_primitives.h> 13 14#include <af/vec2d.h> 15 16#include <math.h> 17 18 19 20 21 22static const float FULL_ROTATION = 6.28318531; 23 24static inline float degrees_to_radians(float deg) { return ALLEGRO_PI * deg / 180.0f; } 25static inline float radians_to_degrees(float rad) { return 180.0f / ALLEGRO_PI * rad; } 26 27static inline void draw_crosshair(float x, float y, ALLEGRO_COLOR color=al_color_name("white")) 28{ 29 float half_size = 12; 30 al_draw_line(x, y-half_size, x, y+half_size, color, 1.0); 31 al_draw_line(x-half_size, y, x+half_size, y, color, 1.0); 32} 33 34static inline void draw_crosshair(vec2d &point, ALLEGRO_COLOR color=al_color_name("white")) 35{ 36 float half_size = 5; 37 al_draw_line(point.x, point.y-half_size, point.x, point.y+half_size, color, 1.0); 38 al_draw_line(point.x-half_size, point.y, point.x+half_size, point.y, color, 1.0); 39} 40 41static inline float distance(float x1, float y1, float x2, float y2) 42{ 43 return sqrt( ((x1 - x2) * (x1 - x2)) + ((y1 - y2) * (y1 - y2)) ) ; 44} 45 46static inline float distance(const vec2d &point1, const vec2d &point2) 47{ 48 return sqrt( ((point1.x - point2.x) * (point1.x - point2.x)) + ((point1.y - point2.y) * (point1.y - point2.y)) ) ; 49} 50 51static inline float distance(const vec2d *point1, const vec2d *point2) 52{ 53 return sqrt( ((point1->x - point2->x) * (point1->x - point2->x)) + ((point1->y - point2->y) * (point1->y - point2->y)) ) ; 54} 55 56static inline float manhattan_distance(const vec2d *point1, const vec2d *point2) 57{ 58 return abs(point2->x + point2->y + point1->x + point1->y); 59} 60 61static inline float manhattan_distance(float x1, float y1, float x2, float y2) 62{ 63 return abs(x1 + y1 + x2 + y2); 64} 65 66static inline float distance_squared(const vec2d &point1, const vec2d &point2) 67{ 68 return ((point1.x - point2.x) * (point1.x - point2.x)) + ((point1.y - point2.y) * (point1.y - point2.y)); 69} 70 71static inline float random_float(float min, float max) 72{ 73 return ((float) rand()/RAND_MAX)*(max-min) + min; 74} 75 76static inline double random_double(double min, double max) 77{ 78 return ((double) rand()/RAND_MAX)*(max-min) + min; 79} 80 81static inline int random_int(int min, int max) 82{ 83 return rand()%(max-min+1) + min; 84} 85 86static inline bool random_bool() 87{ 88 return (rand()%2 == 1); 89} 90 91static inline int random_sign() 92{ 93 if (random_bool()) return 1; 94 else return -1; 95} 96 97// this random_char() function has not been tested or verified 98/* 99 100static inline char random_char(char min=0, char max=255) 101{ 102 return (char)(rand()%(max-min+1) + min); 103} 104 105*/ 106 107static inline unsigned char random_letter(bool lower) 108{ 109 if (lower) return (unsigned char)(rand()%26 + 'a'); 110 return (unsigned char)(rand()%26 + 'A'); 111} 112 113 114// returns a point projected onto an axis 115static inline vec2d project(vec2d &point, vec2d &axis) 116{ 117 float somethun = (point.x * axis.x + point.y * axis.y) 118 / (pow(axis.x, 2) + pow(axis.y, 2)); 119 return vec2d(somethun * axis.x, somethun * axis.y); 120} 121 122// reflect a point along an axis 123inline static vec2d reflect(vec2d &point, const vec2d &axis) 124{ 125 float d = point * axis; 126 return point - 2 * d * axis; 127} 128 129template<class T> 130inline static T limit(const T &range1, const T &range2, const T &val) 131{ 132 float min = (range1 < range2) ? range1 : range2; 133 float max = (range1 > range2) ? range1 : range2; 134 if (val < min) return min; 135 if (val > max) return max; 136 return val; 137} 138 139template<class T> 140inline static bool in_range(const T &min, const T &max, const T &val) 141{ 142 if (val < min) return false; 143 if (val > max) return false; 144 return true; 145} 146 147static inline ALLEGRO_COLOR color(const char *color_name, float alpha=1.0) 148{ 149 ALLEGRO_COLOR col = al_color_name(color_name); 150 col.a = alpha; 151 col.r *= alpha; 152 col.g *= alpha; 153 col.b *= alpha; 154 return col; 155} 156 157 158static inline ALLEGRO_COLOR color_hex(const char *hex, float a=1.0f) 159{ 160 ALLEGRO_COLOR color = al_color_html(hex); 161 color.a = a; 162 color.r *= a; 163 color.g *= a; 164 color.b *= a; 165 return color; 166} 167 168 169static inline ALLEGRO_COLOR color_name(const char *name, float a=1.0f) 170{ 171 ALLEGRO_COLOR color = al_color_name(name); 172 color.a = a; 173 color.r *= a; 174 color.g *= a; 175 color.b *= a; 176 return color; 177} 178 179 180static inline ALLEGRO_COLOR color(ALLEGRO_COLOR color, float a=1.0f) 181{ 182 color.a *= a; 183 color.r *= a; 184 color.g *= a; 185 color.b *= a; 186 return color; 187} 188 189 190static inline bool operator ==(const ALLEGRO_COLOR &lhs, const ALLEGRO_COLOR &rhs) 191{ 192 if (lhs.r != rhs.r) return false; 193 if (lhs.g != rhs.g) return false; 194 if (lhs.b != rhs.b) return false; 195 if (lhs.a != rhs.a) return false; 196 return true; 197} 198 199 200template<class T> 201static inline std::string tostring(T val) 202{ 203 std::ostringstream s; 204 s << val; 205 return s.str(); 206} 207 208 209#include <vector> 210static inline std::vector<std::string> str_explode(const std::string &delimiter, const std::string &str) 211{ 212 std::vector<std::string> arr; 213 214 int strleng = str.length(); 215 int delleng = delimiter.length(); 216 if (delleng==0) 217 return arr;//no change 218 219 int i=0; 220 int k=0; 221 while(i<strleng) 222 { 223 int j=0; 224 while (i+j<strleng && j<delleng && str[i+j]==delimiter[j]) 225 j++; 226 if (j==delleng)//found delimiter 227 { 228 arr.push_back(str.substr(k, i-k)); 229 i+=delleng; 230 k=i; 231 } 232 else 233 { 234 i++; 235 } 236 } 237 arr.push_back(str.substr(k, i-k)); 238 return arr; 239} 240 241 242 243// Tested, but not thoroughly 244#include <string> 245static inline std::string str_replace(const std::string &search, const std::string &replace, std::string &subject) 246{ 247 std::string buffer; 248 249 int sealeng = search.length(); 250 int strleng = subject.length(); 251 252 if (sealeng==0) 253 return subject;//no change 254 255 for(int i=0, j=0; i<strleng; j=0 ) 256 { 257 while (i+j<strleng && j<sealeng && subject[i+j]==search[j]) 258 j++; 259 if (j==sealeng)//found 'search' 260 { 261 buffer.append(replace); 262 i+=sealeng; 263 } 264 else 265 { 266 buffer.append( &subject[i++], 1); 267 } 268 } 269 subject = buffer; 270 return subject; 271} 272 273 274 275#include <vector> 276static std::vector<int> to_int(const std::vector<std::string> &arr) 277{ 278 std::vector<int> result; 279 for (int i=0; i<(int)arr.size(); i++) 280 result.push_back(atoi(arr[i].c_str())); 281 return result; 282} 283 284 285 286 287#include <fstream> 288#include <string> 289static bool file_exists(std::string filename) 290{ 291 std::fstream file(filename.c_str()); 292 if (!file) return false; 293 return true; 294} 295 296 297 298 299#include <fstream> 300#include <string> 301static std::string file_get_contents(std::string filename) 302{ 303 std::ifstream file(filename.c_str()); 304 std::string input = ""; 305 if (!file) return ""; 306 char ch; 307 while (file.get(ch)) input.append(1, ch); 308 if (!file.eof()) return ""; // strange error 309 file.close(); 310 return input; 311} 312 313 314 315 316#include <fstream> 317#include <iostream> 318#include <string> 319static bool file_put_contents(std::string filename, std::string contents) 320{ 321 std::ofstream file; 322 file.open(filename.c_str()); 323 if (!file.is_open()) return false; 324 file << contents.c_str(); 325 file.close(); 326 return true; 327} 328 329 330 331 332static std::string strtoupper(std::string input) 333{ 334 for (unsigned i=0; i<input.size(); i++) 335 if ((input.at(i) <= 122) && (input.at(i) >= 97)) input[i] = input[i] - 32; 336 return input; 337} 338 339 340 341 342static void invert(ALLEGRO_BITMAP *img) 343{ 344 //if (!img) return; 345 int w = al_get_bitmap_width(img); 346 int h = al_get_bitmap_height(img); 347 ALLEGRO_STATE state; 348 al_store_state(&state, ALLEGRO_STATE_TARGET_BITMAP); 349 al_set_target_bitmap(img); 350 al_lock_bitmap(img, NULL, 0); 351 ALLEGRO_COLOR pix; 352 for (int x=0; x<w; x++) 353 for (int y=0; y<h; y++) 354 { 355 pix = al_get_pixel(img, x, y); 356 al_put_pixel(x, y, al_map_rgba_f(1.0-pix.r, 1.0-pix.g, 1.0-pix.b, pix.a)); 357 } 358 al_unlock_bitmap(img); 359 al_restore_state(&state); 360} 361 362 363 364 365#endif

Arthur Kalliokoski
Second in Command
February 2005
avatar

Well, you defined it, but didn't use it! :P

The reason why that warning hasn't appeared until you used the static keyword is because the function may have been referenced by another file, but the static keyword explicitly makes that impossible. I suppose they warn you about "unused" in case you absentmindedly forgot.

“Throughout history, poverty is the normal condition of man. Advances which permit this norm to be exceeded — here and there, now and then — are the work of an extremely small minority, frequently despised, often condemned, and almost always opposed by all right-thinking people. Whenever this tiny minority is kept from creating, or (as sometimes happens) is driven out of a society, the people then slip back into abject poverty. This is known as "bad luck.”

― Robert A. Heinlein

Mark Oates
Member #1,146
March 2001
avatar

Is there really any substantial (speed? maybe?) benefit to having the function static inside each object?

Is static only so you can privately use the function inside the object without worrying about external conflicts?

Arthur Kalliokoski
Second in Command
February 2005
avatar

The compiler can optimize a static function according to whatever is required of it by the caller, without worrying about every possible side effect. It may inline it without being asked, as well, without having a callable procedure in memory at the same time. Some compilers can optimize globally by looking at all files at once, so to speak, which reduces the effectiveness of static for optimization. Another benefit is avoiding name space pollution. That's all I know.

“Throughout history, poverty is the normal condition of man. Advances which permit this norm to be exceeded — here and there, now and then — are the work of an extremely small minority, frequently despised, often condemned, and almost always opposed by all right-thinking people. Whenever this tiny minority is kept from creating, or (as sometimes happens) is driven out of a society, the people then slip back into abject poverty. This is known as "bad luck.”

― Robert A. Heinlein

Matthew Leverton
Supreme Loser
January 1999
avatar

Something like:

#define INLINE static inline __attribute__ ((used)) 

INLINE void foo(void) {}

Edit: actually you probably just want the "unused" attribute, since it only affects the warning.

Mark Oates
Member #1,146
March 2001
avatar

Am I correct in feeling the urge to make these functions static inline in the first place?

Matthew Leverton
Supreme Loser
January 1999
avatar

Smaller code (i.e., not inline) can be faster if it plays nicer with the cache.

Benchmark it with and without inline functions to see.

Thomas Fjellstrom
Member #476
June 2000
avatar

That really depends though. Optimization is a bit of a dark art. It is possible that your code will get smaller if stuff gets inlined, and optimized into the surrounding code and sharing all kinds of data and statements. And a function call really doesn't help much with speed, I really doubt it'll help not inlining code when that code is being called in a tight loop. And I KNOW it won't do jack if its not being called in a tight loop. If its large code, its going to kill the cache regardless if its inlined or not.

--
Thomas Fjellstrom - [website] - [email] - [Allegro Wiki] - [Allegro TODO]
"If you can't think of a better solution, don't try to make a better solution." -- weapon_S
"The less evidence we have for what we believe is certain, the more violently we defend beliefs against those who don't agree" -- https://twitter.com/neiltyson/status/592870205409353730

Audric
Member #907
January 2001

I may be missing something obvious, but... why did you make them static inline, instead of just inline ?

Arthur Kalliokoski
Second in Command
February 2005
avatar

Audric said:

why did you make them static inline, instead of just inline ?

Because when it's just inline, the compiler has to make a callable routine in case some other source file calls it even if it uses inline code in that particular source file.

“Throughout history, poverty is the normal condition of man. Advances which permit this norm to be exceeded — here and there, now and then — are the work of an extremely small minority, frequently despised, often condemned, and almost always opposed by all right-thinking people. Whenever this tiny minority is kept from creating, or (as sometimes happens) is driven out of a society, the people then slip back into abject poverty. This is known as "bad luck.”

― Robert A. Heinlein

Audric
Member #907
January 2001

Arthur said:

the compiler has to make a callable routine in case some other source file calls it

Ah, I can understand that it's a problem if the linker can't detect that all those callable routines (There will be one in each .o, ie. one for each .cpp that #includ-ed "useful_header.hpp") are actually the same, no ambiguity.

If the linker indeed complains (multiple definitions of...), one way to solve it is to make all callers except one use an "extern inline" definition, and a single one use a "(non-static) inline".
I'd do this using a bit of preprocessor magic, for example:

#ifdef NON_EXTERN
  #define INLINE_PREFIX inline
#else
  #define INLINE_PREFIX extern inline
#endif
...
INLINE_PREFIX float degrees_to_radians(float deg) { return ALLEGRO_PI * deg / 180.0f;

type568
Member #8,381
March 2007
avatar

I find it really useful to inline methods of small classes to avoid the mess of making a source file for'em ;D

Go to: