#4822 closed defect (fixed)
COMMON: Compile error - SWAP undeclared in algorithm.h
Reported by: | salty-horse | Owned by: | lordhoto |
---|---|---|---|
Priority: | normal | Component: | --Other-- |
Version: | Keywords: | ||
Cc: | Game: |
Description
While trying tom compile from SVN with clang, I encountered the following error:
In file included from engines/scumm/detection.cpp:26: In file included from ./base/plugins.h:31: In file included from ./common/util.h:30: In file included from ./common/str.h:29: In file included from ./common/array.h:29: ./common/algorithm.h:182:2: error: use of undeclared identifier 'SWAP'
algorithm.h uses SWAP without including util.h.
The reason the code compiles in gcc is that (after preprocessing) engines/scumm/detection.cpp includes common/util.h AFTER including common/algorithm.h. Since SWAP depends on the template arguments of sortPartition, gcc only instantiates it (and lookups its symbols) at the end of the translation unit.
See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29131 for details. There is an open issue about whether this is defined behavior with the C++ standards committee.
In any case, this is an ugly edge-case. However, it cannot easily be fixed by including common/util.h in common/algorithm.h. This causes a circular dependency between util.h -> str.h -> array.h -> algorithm.h -> util.h
A possible solution is to not use SWAP in algorithm.h.
Ticket imported from: #2971226. Ticket imported from: bugs/4822.
Change History (7)
comment:1 by , 15 years ago
comment:2 by , 15 years ago
Summary: | Compilation error - SWAP undeclared in algorithm.h → COMMON: Compile error - SWAP undeclared in algorithm.h |
---|
comment:4 by , 15 years ago
After including common/util.h in common/algorithm.h, clang++ successfully compiles the code.
comment:5 by , 15 years ago
Just added the common/util.h include in common/algorithm.h.
Anyway I'm all for splitting up util.h as Fingolfin suggested. We might even consider moving SWAP to algorithm.h (and along with it renaming it to "swap", since it's a function and not a macro). Sames goes for Common::MIN, Common::MAX btw.
I'm not sure whether we should move ABS and CLIP to algorithm.h too, but IMHO they should also be renamed to be all lowercase too.
comment:6 by , 15 years ago
Owner: | set to |
---|---|
Resolution: | → fixed |
Status: | new → closed |
comment:7 by , 6 years ago
Component: | → --Other-- |
---|
That str.h depends on array.h is an old hack, and should be fixed, by moving the definition of StringList to a better place. E.g. a new header file common/str-list.h. We might also want to rename it to StringArray at the same time. That would break the dependency cycle you mention.
However, we also should consider splitting up common/util.h, which contains a jumbled mix of things which accumulated there over the years. This goes beyond this bug report, of course, but here are some ideas for that anyway: * move the RandomSource stuff to common/random.h * get rid of the textconsole.h include (I only put it there to ease transition) * move the string tokenizer to its own file(s) * ...