#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 , 14 years ago
comment:2 by , 14 years ago
| Summary: | Compilation error - SWAP undeclared in algorithm.h → COMMON: Compile error - SWAP undeclared in algorithm.h |
|---|
comment:4 by , 14 years ago
After including common/util.h in common/algorithm.h, clang++ successfully compiles the code.
comment:5 by , 14 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 , 14 years ago
| Owner: | set to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
comment:7 by , 5 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) * ...