win: Make v8 build with /Zc:twoPhase, and improve a comment.
The C++ standard says that template functions should be parsed immediately, and only type-dependent things should be deferred. cl.exe (MSVC's compiler) instead deferred parsing of all template functions until the end of the translation unit, and unreferenced template functions are not parsed at all. clang-cl emulates cl.exe's behavior. Recently, cl.exe (and clang-cl) grew a /Zc:twoPhase flag that opts in to the standards-conforming behavior, and system headers are now clean enough to build with this flag set. This cleans up v8 to also build with this flag. There was just a single issue: RecyclingZoneAllocator() is unused and contains invalid code: It calls the superclass ctor using `ZoneAllocator(nullptr, nullptr)`, when it should be doing `ZoneAllocator<T>(nullptr, nullptr)`. With /Zc:twoPhase, this is now a parsing error. However, since the RecyclingZoneAllocator() default constructor isn't used anywhere, just delete it. Finally, improve the comment for ZoneAllocator's default constructor to explain why it's needed on Windows. Bug: chromium:969702 Change-Id: I7a516afde67fe090a512d7c7214a3c6932754aca Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1652503 Auto-Submit: Nico Weber <thakis@chromium.org> Commit-Queue: Sigurd Schneider <sigurds@chromium.org> Reviewed-by: Sigurd Schneider <sigurds@chromium.org> Cr-Commit-Position: refs/heads/master@{#62108}
This commit is contained in:
parent
0ebc2b6d76
commit
c6e6c2c6c5
@ -26,8 +26,18 @@ class ZoneAllocator {
|
||||
using other = ZoneAllocator<O>;
|
||||
};
|
||||
|
||||
#ifdef V8_CC_MSVC
|
||||
// MSVS unfortunately requires the default constructor to be defined.
|
||||
#ifdef V8_OS_WIN
|
||||
// The exported class ParallelMove derives from ZoneVector, which derives
|
||||
// from std::vector. On Windows, the semantics of dllexport mean that
|
||||
// a class's superclasses that are not explicitly exported themselves get
|
||||
// implicitly exported together with the subclass, and exporting a class
|
||||
// exports all its functions -- including the std::vector() constructors
|
||||
// that don't take an explicit allocator argument, which in turn reference
|
||||
// the vector allocator's default constructor. So this constructor needs
|
||||
// to exist for linking purposes, even if it's never called.
|
||||
// Other fixes would be to disallow subclasses of ZoneVector (etc) to be
|
||||
// exported, or using composition instead of inheritance for either
|
||||
// ZoneVector and friends or for ParallelMove.
|
||||
ZoneAllocator() : ZoneAllocator(nullptr) { UNREACHABLE(); }
|
||||
#endif
|
||||
explicit ZoneAllocator(Zone* zone) : zone_(zone) {}
|
||||
@ -84,13 +94,6 @@ class RecyclingZoneAllocator : public ZoneAllocator<T> {
|
||||
using other = RecyclingZoneAllocator<O>;
|
||||
};
|
||||
|
||||
#ifdef V8_CC_MSVC
|
||||
// MSVS unfortunately requires the default constructor to be defined.
|
||||
RecyclingZoneAllocator()
|
||||
: ZoneAllocator(nullptr, nullptr), free_list_(nullptr) {
|
||||
UNREACHABLE();
|
||||
}
|
||||
#endif
|
||||
explicit RecyclingZoneAllocator(Zone* zone)
|
||||
: ZoneAllocator<T>(zone), free_list_(nullptr) {}
|
||||
template <typename U>
|
||||
|
Loading…
Reference in New Issue
Block a user