[cfe-commits] [PATCH] [libcxx] cleanup a few compile warnings emitted by GCC
Howard Hinnant
hhinnant at apple.com
Thu Dec 27 10:49:06 PST 2012
On Dec 25, 2012, at 11:54 AM, Saleem Abdulrasool <compnerd at compnerd.org> wrote:
> This just rounds up a few compile warnings emitted by GCC (4.7.2). Included
> are:
> - missing field initializers
> - extraneous semicolon
> - unused variable
> - explicit instantiation of base class
> - comparisons between signed and unsigned values
> - array indexing via a char type
>
> http://llvm-reviews.chandlerc.com/D242
>
> Files:
> src/debug.cpp
> src/locale.cpp
> src/thread.cpp
> <D242.1.patch>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Partially accepted as noted below, committed revision 171165. Thanks!
> Index: src/debug.cpp
> ===================================================================
> --- src/debug.cpp
> +++ src/debug.cpp
> @@ -23,7 +23,7 @@
> {
> static __libcpp_db db;
> return &db;
> -};
> +}
Ok, but src/debug.cpp is currently not working, and not being worked on.
> _LIBCPP_VISIBLE
> const __libcpp_db*
> Index: src/locale.cpp
> ===================================================================
> --- src/locale.cpp
> +++ src/locale.cpp
> @@ -207,7 +207,8 @@
> }
>
> locale::__imp::__imp(const __imp& other)
> - : facets_(max<size_t>(N, other.facets_.size())),
> + : facet(),
> + facets_(max<size_t>(N, other.facets_.size())),
> name_(other.name_)
> {
> facets_ = other.facets_;
Rejected. I prefer to not write code that the compiler will write for me. If you need to insert a pragma to silence a warning, that would be my preferred way to deal with that.
> @@ -4583,7 +4584,7 @@
> string
> __time_get_storage<char>::__analyze(char fmt, const ctype<char>& ct)
> {
> - tm t = {0};
> + tm t = {0,0,0,0,0,0,0,0,0,0,0};
> t.tm_sec = 59;
> t.tm_min = 55;
> t.tm_hour = 23;
> @@ -4729,7 +4730,7 @@
> wstring
> __time_get_storage<wchar_t>::__analyze(char fmt, const ctype<wchar_t>& ct)
> {
> - tm t = {0};
> + tm t = {0,0,0,0,0,0,0,0,0,0,0};
> t.tm_sec = 59;
> t.tm_min = 55;
> t.tm_hour = 23;
Rejected. tm contains *at least* 9 int data members. The portable and concise way to zero initialize this struct is with the single {0}.
> @@ -4746,7 +4747,7 @@
> strftime_l(buf, 100, f, &t, __loc_);
> wchar_t wbuf[100];
> wchar_t* wbb = wbuf;
> - mbstate_t mb = {0};
> + mbstate_t mb = {0,{0}};
> const char* bb = buf;
> #ifdef _LIBCPP_LOCALE__L_EXTENSIONS
> size_t j = mbsrtowcs_l( wbb, &bb, sizeof(wbuf)/sizeof(wbuf[0]), &mb, __loc_);
Rejected. The internal structure of mbstate_t is not known. There is no need for this code to assume an internal structure. The portable and concise way to zero initialize this struct is with the single {0}.
Index: src/thread.cpp
> ===================================================================
> --- src/thread.cpp
> +++ src/thread.cpp
> @@ -67,7 +67,7 @@
> return n;
> #elif defined(_POSIX_C_SOURCE) && (_POSIX_C_SOURCE >= 200112L) && defined(_SC_NPROCESSORS_ONLN)
> long result = sysconf(_SC_NPROCESSORS_ONLN);
> - if (result < 0 || result > UINT_MAX)
> + if (result < 0 || result > static_cast<long>(UINT_MAX))
> result = 0;
> return result;
> #else // defined(CTL_HW) && defined(HW_NCPU)
>
Tentatively rejected. This needs review by someone on this platform. The code looks problematic to me when long and int are both 32 bit.
Howard
More information about the cfe-commits
mailing list