Skip to content
This repository was archived by the owner on Jun 30, 2025. It is now read-only.

Comments

Scope macros containing var initialisations#71

Closed
eelstork wants to merge 2 commits intogoogle:masterfrom
eelstork:patch-1
Closed

Scope macros containing var initialisations#71
eelstork wants to merge 2 commits intogoogle:masterfrom
eelstork:patch-1

Conversation

@eelstork
Copy link

With some compilers declaring and initialising variables inside a switch may cause errors.
A real world example here.
Scoping these declarations solves the problem with hopefully no unwanted side effects.

@eelstork eelstork changed the title Scope macros containing static var initialisations Scope macros containing var initialisations Nov 23, 2015
@longjon
Copy link

longjon commented Nov 23, 2015

Note that these should probably use do...while(0) blocks, see e.g. http://stackoverflow.com/questions/1067226/c-multi-line-macro-do-while0-vs-scope-block.

@eelstork
Copy link
Author

@longjon is right. Updated.

@ukai
Copy link
Contributor

ukai commented Nov 27, 2015

build fails.
g++ -DHAVE_CONFIG_H -I. -I./src -I./src -pthread -Wall -Wwrite-strings -Wo
verloaded-virtual -Wno-sign-compare -DNO_FRAME_POINTER -g -O2 -MT src/logging_
unittest-logging_unittest.o -MD -MP -MF src/.deps/logging_unittest-logging_unitt
est.Tpo -c -o src/logging_unittest-logging_unittest.o test -f 'src/logging_unit test.cc' || echo './'src/logging_unittest.cc
In file included from src/utilities.h:82:0,
from src/logging_unittest.cc:33:
src/logging_unittest.cc: In function ‘void TestLogging(bool)’:
src/glog/logging.h:916:3: error: expected ‘;’ before ‘}’ token
}while(0)
^
src/glog/logging.h:949:3: note: in expansion of macro ‘SOME_KIND_OF_PLOG_EVERY_N

SOME_KIND_OF_PLOG_EVERY_N(severity, (n), google::LogMessage::SendToLog)
^
src/logging_unittest.cc:251:5: note: in expansion of macro ‘PLOG_EVERY_N’
PLOG_EVERY_N(ERROR, 2) << "Plog every 2, iteration " << COUNTER;
^
src/logging_unittest.cc:251:28: error: expected ‘;’ before ‘<<’ token
PLOG_EVERY_N(ERROR, 2) << "Plog every 2, iteration " << COUNTER;
^

@eelstork
Copy link
Author

@ukai thanks for looking at this; would it be meaningful to setup some kind of C.I. to help with similar issues?
Working on this.

@eelstork
Copy link
Author

@ukai since scoping macro contents breaks uses of the insertion operator, I suggest using intermediate variables so that anonymous scopes can be narrowed (demonstrated in latest commit, passes tests). Help is needed regarding proper naming and namespace inclusion of do_log and log_occurrences.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems it makes LOG_EVERY macro thread unsafe?

@eelstork
Copy link
Author

@ukai thanks for reviewing this PR. Yes, I can see how this may affect thread safety.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants