What's going on with C++ macro multiplication

Question

#define MAX 265

std::cout << 0 * MAX << std::endl; //to my surprise, the output is 9 rather than 0

What's the problem with this C++ macro multiplication?

EDIT:

The following is the complete version.

#include <stdio.h>
#include <string.h>
#include <iostream>

#define NAME_BYTES 256
#define VERSION_BYTES 256
#define SIZE_BYTES 32
#define USED_LOCK_COUNT_BYTES 32
#define LOCK_NAME_BYTES 256
#define LOCK_TYPE_BYTES 1
#define PID_BYTES 4
#define TID_BYTES 4
#define LOCK_BYTES LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES 
#define HEADER_BYTES NAME_BYTES + VERSION_BYTES + SIZE_BYTES + USED_LOCK_COUNT_BYTES

int main() {
  std::cout << "LOCK_BYTES: " << LOCK_BYTES << std::endl;
  std::cout << "HEADER_BYTES: " << HEADER_BYTES << std::endl;
  std::cout << "LOCK_BYTES * 0: " << 0 * LOCK_BYTES << std::endl;
}

Here's the result I just got and the compiler info.

yifeng@yifeng-Precision-WorkStation-T3400:~/Shared-Memory-Solution/examples/IMPL$ g++ -v Using built-in specs. COLLECT_GCC=g++ COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.6.1/lto-wrapper Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro 4.6.1-9ubuntu3' --with-bugurl=file:///usr/share/doc/gcc-4.6/README.Bugs --enable-languages=c,c++,fortran,objc,obj-c++,go --prefix=/usr --program-suffix=-4.6 --enable-shared --enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.6 --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-plugin --enable-objc-gc --disable-werror --with-arch-32=i686 --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 4.6.1 (Ubuntu/Linaro 4.6.1-9ubuntu3)

yifeng@yifeng-Precision-WorkStation-T3400:~/Shared-Memory-Solution/examples/IMPL$ ./a.out LOCK_BYTES: 265 HEADER_BYTES: 576 LOCK_BYTES * 0: 9

EDIT: Thank you so much guys!! I've been most happy that I decided to post this, although I am getting so many downvotes. What a lesson to learn about MACRO!

score 13 · Accepted Answer · answered Dec 14 '11 at 17:52

13

You should always put parentheses around macro definitions:

#define LOCK_BYTES (LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES)

Otherwise, the code expands to:

cout << 0 * LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES

which outputs the value of LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES.

Better still, don't use macros unless you really have to. These are better represented as constant variables.

answered Dec 14 '11 at 17:52

Mike Seymour

249,747
28
448
644

Thank you very very very much. I've wasted so much time on this! – Terry Li Dec 14 '11 at 17:54
5

@TerryLiYifeng: Yes, debugging macros can take a lot of time. You're much better off avoiding them except in the very rare cases when neither constants, functions nor templates can do what you want. – Mike Seymour Dec 14 '11 at 17:56

Xeo · Answer 2 · 2011-12-14T18:00:00.940

9

std::cout << "LOCK_BYTES * 0: " << 0 * LOCK_BYTES << std::endl;

Expands to

std::cout << "LOCK_BYTES * 0: " << 0 * LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES << std::endl;

Which in turn expands to

std::cout << "LOCK_BYTES * 0: " << 0 * 256 + 1 + 4 + 4 << std::endl;

And with added parens for the precedence rules:

std::cout << "LOCK_BYTES * 0: " << ((((0 * 256) + 1) + 4) + 4) << std::endl;

Which evaluates to

std::cout << "LOCK_BYTES * 0: " << 9 << std::endl;

Change your code to

std::cout << "LOCK_BYTES * 0: " << 0 * (LOCK_BYTES) << std::endl;

Or even better, use const unsigned int values:

const unsigned int NAME_BYTES = 256;
const unsigned int VERSION_BYTES = 256;
const unsigned int SIZE_BYTES = 32;
const unsigned int USED_LOCK_COUNT_BYTES = 32;
const unsigned int LOCK_NAME_BYTES = 256;
const unsigned int LOCK_TYPE_BYTES = 1;
const unsigned int PID_BYTES = 4;
const unsigned int TID_BYTES = 256;
const unsigned int LOCK_BYTES = LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES;
const unsigned int HEADER_BYTES = NAME_BYTES + VERSION_BYTES + SIZE_BYTES + USED_LOCK_COUNT_BYTES;

Huzzah! And suddenly, you don't have weird problems anymore.

edited Dec 14 '11 at 18:00

answered Dec 14 '11 at 17:53

Xeo

129,499
52
291
397

2

+1 for the `const` discussion. I would like to also award -1 for the ALL UPPERCASE NAMES. But I'm tired, one voting is quite enough... Cheers, and please don't use them ALL UPPERCASE NAMES, because it's like SHOUTING and it also conflicts with the convention for macro names, – Cheers and hth. - Alf Dec 14 '11 at 18:03
@Alf: In my convention, ALL UPPERCASE NAMES stand for whole-program-constants in my code, whether they are macros or not. :) – Xeo Dec 14 '11 at 18:04
2

that's Java convention, it does not work well in C++. Java got it from C without the Java folks understanding it. it is of course your choice to use that very ill-fitting convention in C++, but as long you're doing it, and ignore e.g. the FAQ lite, Bjarne's FAQ, my advice here, etc., you know or at least will have to suspect that there is something very very basic that you have failed to understand. Cheers, – Cheers and hth. - Alf Dec 14 '11 at 18:09
@Alf: Great thing I never coded in Java! From the beginning to now, I always coded in C++. And now, what would be that "something very very basic" that I failed to understand? – Xeo Dec 14 '11 at 18:13
1

@Alf: Does the C++ FAQ suggest this? – Oliver Charlesworth Dec 14 '11 at 18:14
@Oli: if you mean the FAQ Lite, yes by example. including changing a macro to uppercase to avoid name conflict. Bjarne's FAQ is more explict. Coding guidelines are even more explicit. At one time only utter newbies were not aware of this, but education has failed. – Cheers and hth. - Alf Dec 14 '11 at 18:20
@Xeo: That "something very very basic" you failed to understand is that the ALL_UPPERCASE_IDENTIFIERS convention came from the desire to separate the macro namespace from the rest of the language (because macros trample over everything) and has no useful purpose other than that. I find `LOCK_NAME_BYTES` an eyesore and much harder to read than `lock_name_bytes` (and thus appropriate for macros as a reminder of applying something evil). If you need a convention for global constants, pick any of the many other available ones (like `k_lock_name_bytes`), but get rid of ALL_UPPERCASE_IDENTIFIERS. – sbi Dec 15 '11 at 09:57
@sbi: Thanks for the explanation. That said, since I don't code under any convention but my own on my hobby projects, I can freely choose my coding style. For me, it's easier to seperate `GLOBALS`, `locals_and_parameters` and `_member_variables` by giving their names different styles. Until I have to work under a convention with others again, I'll continue this style. :) Last but not least, the coding style used should not influence votings; they should be based on correctness. Anybody can change the names if they want to. – Xeo Dec 15 '11 at 14:04
@sbi: If it's really such an eyesore, you are free to go ahead and "improve" my answer with an edit. – Xeo Dec 15 '11 at 14:06
@Xeo: First: Over the years I have learned to switch naming conventions when switching between companies, projects, or even just source files. In your professional career you will bump into many of them and will have to learn to do that, too. So the inflexibility you display now, as a student, might actually hurt you later. Second: Of course, programming style _does_ influence voting, and it _should_ do so. (You would not upvote answers with outrageous indentation, would you?) Finally: I already did make those constants lower-case. See [here](http://stackoverflow.com/a/8509319/140719). – sbi Dec 15 '11 at 16:26
@sbi: I never said I'm not willing to change my coding style when I'm working together with others on a project or when a project has predefined coding conventions. I even adapt my coding style to that of the questions on SO most of the time. I only said that I have my own conventions for *my own hobby projects*. – Xeo Dec 15 '11 at 16:46

ApprenticeHacker · Answer 3 · 2011-12-14T18:01:18.757

6

I think you've miscalculated a bit. macros do not act like variables, their values are inserted in their place before compilation by the preprocessor. So therefore, what the compiler will see is:

 0* LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES

which is:

 0 * 256 + 1 + 4 + 4

which according to the Order of Operations, on which C++'s operator precedence is based, multiplication happens first, so it will equal 9 not 0.

P.S If you are not developing for embedded systems or obsolete consoles like the Gameboy Color (notice my gravatar), I strongly recommend you to use the const keyword rather than #defines for these kind of things.

edited Dec 14 '11 at 18:01

answered Dec 14 '11 at 17:37

ApprenticeHacker

21,351
27
103
153

7

This is not an answer, it's a *comment*. – Puppy Dec 14 '11 at 17:39
The thing I'm answering isn't a question. – ApprenticeHacker Dec 14 '11 at 17:42
4

That doesn't justify posting non-answers. – Puppy Dec 14 '11 at 17:42
3

@DeadMG: Funnily another "works-for-me" answer has received upvotes. This answer is even better: may-be asker is indeed misreading 0 in the console font. – UncleBens Dec 14 '11 at 17:46
God bless the person who upvoted me. Though I agree with @DeadMG – ApprenticeHacker Dec 14 '11 at 17:47
2

@UncleBens: Not from me. It probably only received those votes because it was sooner. It's equally wrong. Neither of them are "answers", they're worthless. – Puppy Dec 14 '11 at 17:48
and the person who answered this one is probably a jerk who hasn't even read the question properly. – ApprenticeHacker Dec 14 '11 at 17:50

score 5 · Answer 4 · edited May 23 '17 at 10:09

The problem is that you use macros. Your

#define LOCK_BYTES LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES

doesn't do what you think. What it does is to textually replace every occurance of LOCK_BYTES with LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES. So

0 * LOCK_BYTES

expands to

0 * LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES

This is C++. Avoid macros whenever possible. We have const for that.

This works just fine for me:

#include <iostream>

const int name_bytes = 256;
const int version_bytes = 256;
const int size_bytes = 32;
const int used_lock_count_bytes = 32;
const int lock_name_bytes = 256;
const int lock_type_bytes = 1;
const int pid_bytes = 4;
const int tid_bytes = 4;
const int lock_bytes = lock_name_bytes + lock_type_bytes + pid_bytes + tid_bytes;
const int header_bytes = name_bytes + version_bytes + size_bytes + used_lock_count_bytes;

int main() {
  std::cout << "lock_bytes: " << lock_bytes << std::endl;
  std::cout << "header_bytes: " << header_bytes << std::endl;
  std::cout << "lock_bytes * 0: " << 0 * lock_bytes << std::endl;
}

Do you have a good C++ book to learn from? You should.

score 2 · Answer 5 · answered Dec 14 '11 at 17:53

std::cout << "LOCK_BYTES * 0: " << 0 * LOCK_BYTES << std::endl;

expands to

std::cout << 0 * LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES << std::endl;

which, from the basic order of operations, is not equivalent to 0 * (that whole thing). Always enclose expressions inside macro definitions in parenthesis to avoid this sort of error — remember that preprocessor expands macros (more or less) literally.

score 1 · Answer 6 · answered Dec 14 '11 at 17:56

Posting for completeness:

const unsigned NAME_BYTES = 256;
const unsigned VERSION_BYTES = 256;
const unsigned SIZE_BYTES = 32;
const unsigned USED_LOCK_COUNT_BYTES = 32;
const unsigned LOCK_NAME_BYTES = 256;
const unsigned LOCK_TYPE_BYTES = 1;
const unsigned PID_BYTES = 4;
const unsigned TID_BYTES = 4;
const unsigned LOCK_BYTES = LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES;
const unsigned HEADER_BYTES = NAME_BYTES + VERSION_BYTES + SIZE_BYTES + USED_LOCK_COUNT_BYTES;

Macros are expanded, consts aren't. Always prefer consts as they are type safe and don't have paren problems.

What's going on with C++ macro multiplication

6 Answers6

Linked