to static or not to static?

no-reply@allegro.cc (Mark Oates) — Tue, 27 Dec 2011 09:17:19 +0000

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:

just jump the shark and make them regular functions that exist in a single object file, or
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.
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 
   8#include 
   9
  10#include 
  11#include 
  12#include 
  13
  14#include 
  15
  16#include 
  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 
 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 
 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 
 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 
 288#include 
 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 
 300#include 
 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 
 317#include 
 318#include 
 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

no-reply@allegro.cc (Arthur Kalliokoski) — Tue, 27 Dec 2011 09:20:16 +0000

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

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.

no-reply@allegro.cc (Mark Oates) — Tue, 27 Dec 2011 09:30:32 +0000

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?

no-reply@allegro.cc (Arthur Kalliokoski) — Tue, 27 Dec 2011 09:35:14 +0000

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.

no-reply@allegro.cc (Matthew Leverton) — Tue, 27 Dec 2011 09:37:51 +0000

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.

no-reply@allegro.cc (Mark Oates) — Tue, 27 Dec 2011 09:53:30 +0000

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

no-reply@allegro.cc (Matthew Leverton) — Tue, 27 Dec 2011 10:20:21 +0000

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.

no-reply@allegro.cc (Thomas Fjellstrom) — Tue, 27 Dec 2011 10:25:47 +0000

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.

no-reply@allegro.cc (Audric) — Tue, 27 Dec 2011 16:53:38 +0000

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

no-reply@allegro.cc (Arthur Kalliokoski) — Tue, 27 Dec 2011 18:41:57 +0000

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.

no-reply@allegro.cc (Audric) — Tue, 27 Dec 2011 19:11:41 +0000

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;

no-reply@allegro.cc (type568) — Wed, 04 Jan 2012 06:37:07 +0000

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