Issue 552 in protobuf: PC-Lint accusing errors in atomicops.h and type_traits.h

[prot...@googlecode.com]

Status: New
Owner: ----
Labels: Type-Defect Priority-Medium

New issue 552 by Felipe.F...@gmail.com: PC-Lint accusing errors in
atomicops.h and type_traits.h
http://code.google.com/p/protobuf/issues/detail?id=552

When checking a source file that includes the protocol buffer library
v2.5.0 with PC-Lint 9.00j in VS2010 the following two errors happen:

1) atomicops.h:161:
// Include our platform specific implementation.
#define GOOGLE_PROTOBUF_ATOMICOPS_ERROR \
#error "Atomic operations are not supported on your platform"

-- PC-Lint does not recognize the #error directive inside the #define.

2) type_traits.h:105:
#if defined(_MSC_VER)
// wchar_t is not by default a distinct type from unsigned short in
// Microsoft C.
// See http://msdn2.microsoft.com/en-us/library/dh8che7s(VS.80).aspx
template<> struct is_integral<__wchar_t> : true_type { };
#else
template<> struct is_integral<wchar_t> : true_type { };
#endif

-- PC-Lint does not have the type __wchar_t defined.



For error number 1), it seems that PC-Lint is right in accusing an
preprocessor error and that MSVC just allows that code by chance. There is
a similar case when trying to use #pragma warning directives inside a macro
definition, where one should use __pragma directives (c.f.
http://msdn.microsoft.com/en-us/library/d9x1s805.aspx). Since #error does
not have the __error version, one possible solution would be eliminating
the #define GOOGLE_PROTOBUF_ATOMICOPS_ERROR and just simply copy paste the
#error directive where the symbol GOOGLE_PROTOBUF_ATOMICOPS_ERROR is used.
This would have a low-impact since the symbol is used only three times in
the same header where it is defined.


For error number 2), PC-Lint is wrong in not defining the type __wchar_t.
But looking at the practical side, it seems that the type __wchar_t is not
a widely used type, since even microsoft headers don't use it (actually
just one C++/CLI header uses it). It was defined in the compiler just to be
the type that wchar_t will map to when a compiler option is specified. Even
the implementation of is_integral in VS2010's STL doesn't use it directly,
what it does is this:
#ifdef _NATIVE_WCHAR_T_DEFINED
template<>
struct _Is_integral<wchar_t>
: true_type
{ // determine whether _Ty is integral
};
#endif /* _NATIVE_WCHAR_T_DEFINED */

So only when wchar_t is mapped to __wchar_t, then we will define
_Is_integral to it.

For better conformance with the is_integral define in the STL, I would
suggest using:
#if !defined(_MSC_VER) || defined(_NATIVE_WCHAR_T_DEFINED)
template<> struct is_integral<wchar_t> : true_type { };
#endif

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[prot...@googlecode.com]

Comment #1 on issue 552 by xiaof...@google.com: PC-Lint accusing errors in

atomicops.h and type_traits.h
http://code.google.com/p/protobuf/issues/detail?id=552

For 1), I think we can change the definition of
GOOGLE_PROTOBUF_ATOMICOPS_ERROR to:
#define GOOGLE_PROTOBUF_ATOMICOPS_ERROR \

"Atomic operations are not supported on your platform"

and use:
#error GOOGLE_PROTOBUF_ATOMICOPS_ERROR

For 2), it seems you are suggesting we don't define is_integral<__wchar_t>
if wchar_t is not defined as a native type. It sounds wrong to me as users
can still define wchar_t themselves as:
#define wchar_t __wchar_t
and if they do that, is_integral<wchar_t> will return a wrong value if we
don't have a is_integral<__wchar_t> definition.

[prot...@googlecode.com]

Comment #2 on issue 552 by Felipe.F...@gmail.com: PC-Lint accusing errors

in atomicops.h and type_traits.h
http://code.google.com/p/protobuf/issues/detail?id=552

#define wchar_t __wchar_t is a case that even MS doesn't support, if the
user does that MS headers will be inconsistent. They will only work if the
user creates an additional define of _NATIVE_WCHAR_T_DEFINED. Supporting
this cenario is using the compiler in a way that even MS doesn't
use/support.

[prot...@googlecode.com]

Comment #3 on issue 552 by xiaof...@google.com: PC-Lint accusing errors in

atomicops.h and type_traits.h
http://code.google.com/p/protobuf/issues/detail?id=552

I agree with you that maybe __wchar_t is only used by very few people and
we don't have to support it. But as you said it's not wrong for the
type_traits library to have it. The issue should rather be fixed in PC-Lint
to make it support __wchar_t too.