From 5021c4d88506ac19be4302be02c3cd1702f97d03 Mon Sep 17 00:00:00 2001 From: Douglas Heriot Date: Sat, 22 Aug 2015 02:05:40 +1000 Subject: Define GOOGLE_ATTRIBUTE_NOINLINE for MSVC. Workaround for VS2015 Release build compiler bug. See issue #240 - MSVC in VS2015 seems to inline a function it shouldn't. My original workaround was to disable inlining for the whole file, but I found a way to do it on just this specific function using __declspec(noinline). Unfortunately __declspec has to go at the start of the function declaration, while __attribute in GCC can go either before or after. I had to move lots of GOOGLE_ATTRIBUTE_NOLINE to make it compile. I have not yet tested this change with GCC. Will there be other side effects of defining this, given it wasn't previously? I also noticed a few functions marked with both the 'inline' keyword, and GOOGLE_ATTRIBUTE_NOINLINE - huh? Is there an explanation for this, or is it an oversight? --- src/google/protobuf/arena.h | 9 ++++----- src/google/protobuf/arenastring.h | 9 +++++---- src/google/protobuf/message.cc | 5 +++++ src/google/protobuf/metadata.h | 2 +- src/google/protobuf/repeated_field.h | 26 +++++++++++++------------- src/google/protobuf/stubs/port.h | 3 +++ 6 files changed, 31 insertions(+), 23 deletions(-) (limited to 'src') diff --git a/src/google/protobuf/arena.h b/src/google/protobuf/arena.h index 51149bae..e7e693b2 100644 --- a/src/google/protobuf/arena.h +++ b/src/google/protobuf/arena.h @@ -425,16 +425,16 @@ class LIBPROTOBUF_EXPORT Arena { // of the underlying blocks. The total space used may not include the new // blocks that are allocated by this arena from other threads concurrently // with the call to this method. - uint64 SpaceAllocated() const GOOGLE_ATTRIBUTE_NOINLINE; + GOOGLE_ATTRIBUTE_NOINLINE uint64 SpaceAllocated() const; // As above, but does not include any free space in underlying blocks. - uint64 SpaceUsed() const GOOGLE_ATTRIBUTE_NOINLINE; + GOOGLE_ATTRIBUTE_NOINLINE uint64 SpaceUsed() const; // Frees all storage allocated by this arena after calling destructors // registered with OwnDestructor() and freeing objects registered with Own(). // Any objects allocated on this arena are unusable after this call. It also // returns the total space used by the arena which is the sums of the sizes // of the allocated blocks. This method is not thread-safe. - uint64 Reset() GOOGLE_ATTRIBUTE_NOINLINE; + GOOGLE_ATTRIBUTE_NOINLINE uint64 Reset(); // Adds |object| to a list of heap-allocated objects to be freed with |delete| // when the arena is destroyed or reset. @@ -459,8 +459,7 @@ class LIBPROTOBUF_EXPORT Arena { // will be manually called when the arena is destroyed or reset. This differs // from OwnDestructor() in that any member function may be specified, not only // the class destructor. - void OwnCustomDestructor(void* object, void (*destruct)(void*)) - GOOGLE_ATTRIBUTE_NOINLINE { + GOOGLE_ATTRIBUTE_NOINLINE void OwnCustomDestructor(void* object, void (*destruct)(void*)) { AddListNode(object, destruct); } diff --git a/src/google/protobuf/arenastring.h b/src/google/protobuf/arenastring.h index d829ed91..7bbf6c76 100755 --- a/src/google/protobuf/arenastring.h +++ b/src/google/protobuf/arenastring.h @@ -283,9 +283,9 @@ struct LIBPROTOBUF_EXPORT ArenaStringPtr { private: ::std::string* ptr_; + GOOGLE_ATTRIBUTE_NOINLINE inline void CreateInstance(::google::protobuf::Arena* arena, - const ::std::string* initial_value) - GOOGLE_ATTRIBUTE_NOINLINE { + const ::std::string* initial_value) { // Assumes ptr_ is not NULL. if (initial_value != NULL) { ptr_ = new ::std::string(*initial_value); @@ -296,8 +296,9 @@ struct LIBPROTOBUF_EXPORT ArenaStringPtr { arena->Own(ptr_); } } - inline void CreateInstanceNoArena(const ::std::string* initial_value) - GOOGLE_ATTRIBUTE_NOINLINE { + + GOOGLE_ATTRIBUTE_NOINLINE + inline void CreateInstanceNoArena(const ::std::string* initial_value) { if (initial_value != NULL) { ptr_ = new ::std::string(*initial_value); } else { diff --git a/src/google/protobuf/message.cc b/src/google/protobuf/message.cc index 276d7de5..123bd789 100644 --- a/src/google/protobuf/message.cc +++ b/src/google/protobuf/message.cc @@ -52,6 +52,7 @@ #include #include #include +#include namespace google { namespace protobuf { @@ -466,6 +467,10 @@ struct ShutdownRepeatedFieldRegister { namespace internal { template<> +#if defined(_MSC_VER) && (_MSC_VER >= 1900) +// Note: force noinline to workaround MSVC 2015 compiler bug, issue #240 +GOOGLE_ATTRIBUTE_NOINLINE +#endif Message* GenericTypeHandler::NewFromPrototype( const Message* prototype, google::protobuf::Arena* arena) { return prototype->New(arena); diff --git a/src/google/protobuf/metadata.h b/src/google/protobuf/metadata.h index 30b2a6ee..c5bab0a8 100644 --- a/src/google/protobuf/metadata.h +++ b/src/google/protobuf/metadata.h @@ -143,7 +143,7 @@ class LIBPROTOBUF_EXPORT InternalMetadataWithArena { Arena* arena_; }; - UnknownFieldSet* mutable_unknown_fields_slow() GOOGLE_ATTRIBUTE_NOINLINE { + GOOGLE_ATTRIBUTE_NOINLINE UnknownFieldSet* mutable_unknown_fields_slow() { Arena* my_arena = arena(); Container* container = Arena::Create(my_arena); ptr_ = reinterpret_cast( diff --git a/src/google/protobuf/repeated_field.h b/src/google/protobuf/repeated_field.h index 14f46298..9c84ed98 100644 --- a/src/google/protobuf/repeated_field.h +++ b/src/google/protobuf/repeated_field.h @@ -458,22 +458,21 @@ class LIBPROTOBUF_EXPORT RepeatedPtrFieldBase { void AddAllocatedInternal(typename TypeHandler::Type* value, google::protobuf::internal::false_type); - template + template GOOGLE_ATTRIBUTE_NOINLINE void AddAllocatedSlowWithCopy(typename TypeHandler::Type* value, Arena* value_arena, Arena* my_arena) - GOOGLE_ATTRIBUTE_NOINLINE; - template - void AddAllocatedSlowWithoutCopy(typename TypeHandler::Type* value) - GOOGLE_ATTRIBUTE_NOINLINE; +; + template GOOGLE_ATTRIBUTE_NOINLINE + void AddAllocatedSlowWithoutCopy(typename TypeHandler::Type* value); template typename TypeHandler::Type* ReleaseLastInternal(google::protobuf::internal::true_type); template typename TypeHandler::Type* ReleaseLastInternal(google::protobuf::internal::false_type); - template - inline void SwapFallback(RepeatedPtrFieldBase* other) GOOGLE_ATTRIBUTE_NOINLINE; + template GOOGLE_ATTRIBUTE_NOINLINE + inline void SwapFallback(RepeatedPtrFieldBase* other); inline Arena* GetArenaNoVirtual() const { return arena_; @@ -545,13 +544,13 @@ class GenericTypeHandler { // constructors and destructors. Note that the GOOGLE_ATTRIBUTE_NOINLINE macro // requires the 'inline' storage class here, which is somewhat confusing, but // the compiler does the right thing. + GOOGLE_ATTRIBUTE_NOINLINE static inline GenericType* NewFromPrototype(const GenericType* prototype, - ::google::protobuf::Arena* arena = NULL) - GOOGLE_ATTRIBUTE_NOINLINE { + ::google::protobuf::Arena* arena = NULL) { return New(arena); } - static inline void Delete(GenericType* value, Arena* arena) - GOOGLE_ATTRIBUTE_NOINLINE { + GOOGLE_ATTRIBUTE_NOINLINE + static inline void Delete(GenericType* value, Arena* arena) { if (arena == NULL) { delete value; } @@ -564,8 +563,9 @@ class GenericTypeHandler { } static inline void Clear(GenericType* value) { value->Clear(); } - static inline void Merge(const GenericType& from, GenericType* to) - GOOGLE_ATTRIBUTE_NOINLINE { + + GOOGLE_ATTRIBUTE_NOINLINE + static inline void Merge(const GenericType& from, GenericType* to) { to->MergeFrom(from); } static inline int SpaceUsed(const GenericType& value) { diff --git a/src/google/protobuf/stubs/port.h b/src/google/protobuf/stubs/port.h index 8a5d1a13..a3c53dd9 100644 --- a/src/google/protobuf/stubs/port.h +++ b/src/google/protobuf/stubs/port.h @@ -161,6 +161,9 @@ static const uint64 kuint64max = GOOGLE_ULONGLONG(0xFFFFFFFFFFFFFFFF); // For functions we want to force not inline. // Introduced in gcc 3.1. #define GOOGLE_ATTRIBUTE_NOINLINE __attribute__ ((noinline)) +#elif defined(_MSC_VER) && (_MSC_VER >= 1400) +// Seems to have been around since at least Visual Studio 2005 +#define GOOGLE_ATTRIBUTE_NOINLINE __declspec(noinline) #else // Other compilers will have to figure it out for themselves. #define GOOGLE_ATTRIBUTE_NOINLINE -- cgit v1.2.3