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?
This commit is contained in:
Douglas Heriot 2015-08-22 02:05:40 +10:00
parent 0cb84ee31f
commit 5021c4d885
6 changed files with 31 additions and 23 deletions

View File

@ -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);
}

View File

@ -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 {

View File

@ -52,6 +52,7 @@
#include <google/protobuf/stubs/map_util.h>
#include <google/protobuf/stubs/singleton.h>
#include <google/protobuf/stubs/stl_util.h>
#include <google/protobuf/stubs/port.h>
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<Message>::NewFromPrototype(
const Message* prototype, google::protobuf::Arena* arena) {
return prototype->New(arena);

View File

@ -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<Container>(my_arena);
ptr_ = reinterpret_cast<void*>(

View File

@ -458,22 +458,21 @@ class LIBPROTOBUF_EXPORT RepeatedPtrFieldBase {
void AddAllocatedInternal(typename TypeHandler::Type* value,
google::protobuf::internal::false_type);
template <typename TypeHandler>
template <typename TypeHandler> GOOGLE_ATTRIBUTE_NOINLINE
void AddAllocatedSlowWithCopy(typename TypeHandler::Type* value,
Arena* value_arena,
Arena* my_arena)
GOOGLE_ATTRIBUTE_NOINLINE;
template <typename TypeHandler>
void AddAllocatedSlowWithoutCopy(typename TypeHandler::Type* value)
GOOGLE_ATTRIBUTE_NOINLINE;
;
template <typename TypeHandler> GOOGLE_ATTRIBUTE_NOINLINE
void AddAllocatedSlowWithoutCopy(typename TypeHandler::Type* value);
template <typename TypeHandler>
typename TypeHandler::Type* ReleaseLastInternal(google::protobuf::internal::true_type);
template <typename TypeHandler>
typename TypeHandler::Type* ReleaseLastInternal(google::protobuf::internal::false_type);
template<typename TypeHandler>
inline void SwapFallback(RepeatedPtrFieldBase* other) GOOGLE_ATTRIBUTE_NOINLINE;
template<typename TypeHandler> 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) {

View File

@ -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