Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSVC 2022 17.12.1 configure.py --bootstrap fails on emhash/hash_table8.hpp #2531

Open
zufuliu opened this issue Nov 24, 2024 · 8 comments
Open

Comments

@zufuliu
Copy link
Contributor

zufuliu commented Nov 24, 2024

warning C4456: declaration of 'slot' hides previous local declaration:

const auto slot = _index[bucket].slot & _mask;
//prefetch_heap_block((char*)&_pairs[slot]);
if (EMH_EQHASH(bucket, key_hash)) {
if (EMH_LIKELY(_eq(key, _pairs[slot].first)))
return bucket;
}
if (next_bucket == bucket)
return INACTIVE;
while (true) {
if (EMH_EQHASH(next_bucket, key_hash)) {
const auto slot = _index[next_bucket].slot & _mask;

const auto slot = _index[bucket].slot & _mask;
//prefetch_heap_block((char*)&_pairs[slot]);
if (EMH_EQHASH(bucket, key_hash)) {
if (EMH_LIKELY(_eq(key, _pairs[slot].first)))
return slot;
}
if (next_bucket == bucket)
return _num_filled;
while (true) {
if (EMH_EQHASH(next_bucket, key_hash)) {
const auto slot = _index[next_bucket].slot & _mask;

full configure.py --bootstrap --platform msvc output:

H:\llvm\ninja>configure.py --bootstrap --platform msvc
bootstrapping ninja...
depfile_parser.cc
lexer.cc
build.cc
build_log.cc
H:\llvm\ninja\src\third_party/emhash/hash_table8.hpp(1340): error C2220: the following warning is treated as an error
H:\llvm\ninja\src\third_party/emhash/hash_table8.hpp(1340): warning C4456: declaration of 'slot' hides previous local declaration
H:\llvm\ninja\src\third_party/emhash/hash_table8.hpp(1329): note: see declaration of 'slot'
H:\llvm\ninja\src\third_party/emhash/hash_table8.hpp(1340): note: the template instantiation context (the oldest one first) is
H:\llvm\ninja\src\build_log.h(101): note: see reference to class template instantiation 'emhash8::HashMap<StringPiece,V,std::hash<StringPiece>,std::equal_to<KeyT>,std::allocator<std::pair<KeyT,ValueT>>,emhash8::DefaultPolicy>' being compiled
        with
        [
            V=BuildLog::LogEntry *,
            KeyT=StringPiece,
            ValueT=BuildLog::LogEntry *
        ]
H:\llvm\ninja\src\third_party/emhash/hash_table8.hpp(1322): note: while compiling class template member function 'emhash8::HashMap<StringPiece,V,std::hash<StringPiece>,std::equal_to<KeyT>,std::allocator<std::pair<KeyT,ValueT>>,emhash8::DefaultPolicy>::size_type emhash8::HashMap<KeyT,ValueT,std::hash<StringPiece>,std::equal_to<KeyT>,std::allocator<std::pair<KeyT,ValueT>>,emhash8::DefaultPolicy>::find_filled_bucket(const KeyT &,uint64_t) noexcept const'
        with
        [
            V=BuildLog::LogEntry *,
            KeyT=StringPiece,
            ValueT=BuildLog::LogEntry *
        ]
H:\llvm\ninja\src\third_party/emhash/hash_table8.hpp(892): note: see the first reference to 'emhash8::HashMap<StringPiece,V,std::hash<StringPiece>,std::equal_to<KeyT>,std::allocator<std::pair<KeyT,ValueT>>,emhash8::DefaultPolicy>::find_filled_bucket' in 'emhash8::HashMap<StringPiece,V,std::hash<StringPiece>,std::equal_to<KeyT>,std::allocator<std::pair<KeyT,ValueT>>,emhash8::DefaultPolicy>::erase'
        with
        [
            V=BuildLog::LogEntry *,
            KeyT=StringPiece,
            ValueT=BuildLog::LogEntry *
        ]
.\src\build_log.cc(376): note: see the first reference to 'emhash8::HashMap<StringPiece,V,std::hash<StringPiece>,std::equal_to<KeyT>,std::allocator<std::pair<KeyT,ValueT>>,emhash8::DefaultPolicy>::erase' in 'BuildLog::Recompact'
        with
        [
            V=BuildLog::LogEntry *,
            KeyT=StringPiece,
            ValueT=BuildLog::LogEntry *
        ]
H:\llvm\ninja\src\third_party/emhash/hash_table8.hpp(1375): warning C4456: declaration of 'slot' hides previous local declaration
H:\llvm\ninja\src\third_party/emhash/hash_table8.hpp(1364): note: see declaration of 'slot'
H:\llvm\ninja\src\third_party/emhash/hash_table8.hpp(1375): note: the template instantiation context (the oldest one first) is
.\src\build_log.cc(102): note: see reference to function template instantiation 'emhash8::HashMap<StringPiece,V,std::hash<StringPiece>,std::equal_to<KeyT>,std::allocator<std::pair<KeyT,ValueT>>,emhash8::DefaultPolicy>::iterator emhash8::HashMap<KeyT,ValueT,std::hash<StringPiece>,std::equal_to<KeyT>,std::allocator<std::pair<KeyT,ValueT>>,emhash8::DefaultPolicy>::find<std::string>(const K &) noexcept' being compiled
        with
        [
            V=BuildLog::LogEntry *,
            KeyT=StringPiece,
            ValueT=BuildLog::LogEntry *,
            K=std::string
        ]
.\src\build_log.cc(102): note: see the first reference to 'emhash8::HashMap<StringPiece,V,std::hash<StringPiece>,std::equal_to<KeyT>,std::allocator<std::pair<KeyT,ValueT>>,emhash8::DefaultPolicy>::find' in 'BuildLog::RecordCommand'
        with
        [
            V=BuildLog::LogEntry *,
            KeyT=StringPiece,
            ValueT=BuildLog::LogEntry *
        ]
H:\llvm\ninja\src\third_party/emhash/hash_table8.hpp(556): note: see reference to function template instantiation 'emhash8::HashMap<StringPiece,V,std::hash<StringPiece>,std::equal_to<KeyT>,std::allocator<std::pair<KeyT,ValueT>>,emhash8::DefaultPolicy>::size_type emhash8::HashMap<KeyT,ValueT,std::hash<StringPiece>,std::equal_to<KeyT>,std::allocator<std::pair<KeyT,ValueT>>,emhash8::DefaultPolicy>::find_filled_slot<K>(const K &) noexcept const' being compiled
        with
        [
            V=BuildLog::LogEntry *,
            KeyT=StringPiece,
            ValueT=BuildLog::LogEntry *,
            K=std::string
        ]
when running:  cl  /nologo /Zi /W4 /WX /wd4530 /wd4100 /wd4706 /wd4244 /wd4512 /wd4800 /wd4702 /wd4819 /wd4127 /wd4355 /wd4091 /GR- /Zc:__cplusplus /wd4267 /DNOMINMAX /D_CRT_SECURE_NO_WARNINGS /D_HAS_EXCEPTIONS=0 /DNINJA_PYTHON="python3.exe" /FS /Ox /DNDEBUG /GL -I. -c .\src\build_log.cc /Fobuild\build_log.obj /Fdbuild\ninja.pdb
Traceback (most recent call last):
  File "H:\llvm\ninja\configure.py", line 558, in <module>
    objs += cxx(name, variables=cxxvariables)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "H:\llvm\ninja\configure.py", line 297, in cxx
    return n.build(built(name + objext), 'cxx', src(name + '.cc'), **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "H:\llvm\ninja\configure.py", line 176, in build
    self._run_command(self._expand(cmd, local_vars))
  File "H:\llvm\ninja\configure.py", line 201, in _run_command
    subprocess.check_call(cmdline, shell=True)
  File "D:\Dev\Python3\Lib\subprocess.py", line 413, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command 'cl  /nologo /Zi /W4 /WX /wd4530 /wd4100 /wd4706 /wd4244 /wd4512 /wd4800 /wd4702 /wd4819 /wd4127 /wd4355 /wd4091 /GR- /Zc:__cplusplus /wd4267 /DNOMINMAX /D_CRT_SECURE_NO_WARNINGS /D_HAS_EXCEPTIONS=0 /DNINJA_PYTHON="python3.exe" /FS /Ox /DNDEBUG /GL -I. -c .\src\build_log.cc /Fobuild\build_log.obj /Fdbuild\ninja.pdb' returned non-zero exit status 2.

other warnings using LLVM clang-cl 19.1.4.

H:\llvm\ninja>configure.py --bootstrap --platform msvc
bootstrapping ninja...
.\src\disk_interface.cc(168,22): warning: cast from 'FARPROC' (aka 'long long (*)()') to 'FunctionType *' (aka 'unsigned char (*)()') converts to incompatible function type [-Wcast-function-type-mismatch]
  168 |     auto* func_ptr = reinterpret_cast<FunctionType*>(
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  169 |         ::GetProcAddress(ntdll_lib, "RtlAreLongPathsEnabled"));
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from .\src\manifest_parser.cc:15:
In file included from .\src\manifest_parser.h:20:
D:\Program Files\Microsoft Visual Studio\2022\VC\Tools\MSVC\14.42.34433\include\memory(3302,9): warning: delete called on non-final 'ManifestParser' that has virtual functions but non-virtual destructor
      [-Wdelete-non-abstract-non-virtual-dtor]
 3302 |         delete _Ptr;
      |         ^
D:\Program Files\Microsoft Visual Studio\2022\VC\Tools\MSVC\14.42.34433\include\memory(3412,13): note: in instantiation of member function 'std::default_delete<ManifestParser>::operator()' requested here
 3412 |             _Mypair._Get_first()(_Mypair._Myval2);
      |             ^
.\src\manifest_parser.cc(30,17): note: in instantiation of member function 'std::unique_ptr<ManifestParser>::~unique_ptr' requested here
   30 | ManifestParser::ManifestParser(State* state, FileReader* file_reader,
      |                 ^
1 warning generated.
.\src\minidump-win32.cc(55,7): warning: cast from 'FARPROC' (aka 'long long (*)()') to 'MiniDumpWriteDumpFunc' (aka 'int (*)(void *, unsigned long, void *, _MINIDUMP_TYPE, _MINIDUMP_EXCEPTION_INFORMATION
      *const, _MINIDUMP_USER_STREAM_INFORMATION *const, _MINIDUMP_CALLBACK_INFORMATION *const)') converts to incompatible function type [-Wcast-function-type-mismatch]
   55 |       (MiniDumpWriteDumpFunc)GetProcAddress(dbghelp, "MiniDumpWriteDump");
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from .\src\ninja.cc:39:
In file included from .\src\build.h:20:
D:\Program Files\Microsoft Visual Studio\2022\VC\Tools\MSVC\14.42.34433\include\memory(3302,9): warning: delete called on non-final 'ManifestParser' that has virtual functions but non-virtual destructor
      [-Wdelete-non-abstract-non-virtual-dtor]
 3302 |         delete _Ptr;
      |         ^
D:\Program Files\Microsoft Visual Studio\2022\VC\Tools\MSVC\14.42.34433\include\memory(3412,13): note: in instantiation of member function 'std::default_delete<ManifestParser>::operator()' requested here
 3412 |             _Mypair._Get_first()(_Mypair._Myval2);
      |             ^
.\src\manifest_parser.h(41,8): note: in instantiation of member function 'std::unique_ptr<ManifestParser>::~unique_ptr' requested here
   41 | struct ManifestParser : public Parser {
      |        ^
1 warning generated.

GCC 14.2 warnings:

H:\llvm\ninja>configure.py --bootstrap --platform mingw
bootstrapping ninja...
.\src\disk_interface.cc: In constructor 'RealDiskInterface::RealDiskInterface()':
.\src\disk_interface.cc:168:22: warning: cast between incompatible function types from 'FARPROC' {aka 'long long int (*)()'} to 'BOOLEAN (*)()' {aka 'unsigned char (*)()'} [-Wcast-function-type]
  168 |     auto* func_ptr = reinterpret_cast<FunctionType*>(
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  169 |         ::GetProcAddress(ntdll_lib, "RtlAreLongPathsEnabled"));
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.\src\util.cc: In function 'std::string GetLastErrorString()':
.\src\util.cc:547:69: warning: format '%d' expects argument of type 'int', but argument 4 has type 'DWORD' {aka 'long unsigned int'} [-Wformat=]
  547 |     snprintf(fallback_msg, sizeof(fallback_msg), "GetLastError() = %d", err);
      |                                                                    ~^   ~~~
      |                                                                     |   |
      |                                                                     int DWORD {aka long unsigned int}
      |                                                                    %ld
wrote build.ninja.

LLVM mingw 19.1.4 warnings:

H:\llvm\ninja>set CXX=D:\Dev\llvm-mingw\bin\clang++.exe

H:\llvm\ninja>configure.py --bootstrap --platform mingw
bootstrapping ninja...
.\src\disk_interface.cc:168:22: warning: cast from 'FARPROC' (aka 'long long (*)()') to 'FunctionType *' (aka 'unsigned char (*)()') converts to incompatible function type [-Wcast-function-type-mismatch]
  168 |     auto* func_ptr = reinterpret_cast<FunctionType*>(
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  169 |         ::GetProcAddress(ntdll_lib, "RtlAreLongPathsEnabled"));
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from .\src\manifest_parser.cc:15:
In file included from .\src/manifest_parser.h:18:
In file included from .\src/parser.h:18:
In file included from D:/Dev/llvm-mingw/include/c++/v1/string:647:
In file included from D:/Dev/llvm-mingw/include/c++/v1/string_view:941:
In file included from D:/Dev/llvm-mingw/include/c++/v1/algorithm:1842:
In file included from D:/Dev/llvm-mingw/include/c++/v1/__algorithm/for_each.h:16:
In file included from D:/Dev/llvm-mingw/include/c++/v1/__ranges/movable_box.h:21:
In file included from D:/Dev/llvm-mingw/include/c++/v1/optional:1294:
In file included from D:/Dev/llvm-mingw/include/c++/v1/memory:944:
In file included from D:/Dev/llvm-mingw/include/c++/v1/__memory/inout_ptr.h:16:
In file included from D:/Dev/llvm-mingw/include/c++/v1/__memory/shared_ptr.h:32:
D:/Dev/llvm-mingw/include/c++/v1/__memory/unique_ptr.h:80:5: warning: delete called on non-final 'ManifestParser' that has virtual functions but non-virtual destructor [-Wdelete-non-abstract-non-virtual-dtor]
   80 |     delete __ptr;
      |     ^
D:/Dev/llvm-mingw/include/c++/v1/__memory/unique_ptr.h:292:7: note: in instantiation of member function 'std::default_delete<ManifestParser>::operator()' requested here
  292 |       __ptr_.second()(__tmp);
      |       ^
.\src\manifest_parser.cc:425:16: note: in instantiation of member function 'std::unique_ptr<ManifestParser>::reset' requested here
  425 |     subparser_.reset(new ManifestParser(state_, file_reader_, options_));
      |                ^
1 warning generated.
.\src\util.cc:547:73: warning: format specifies type 'int' but the argument has type 'DWORD' (aka 'unsigned long') [-Wformat]
  547 |     snprintf(fallback_msg, sizeof(fallback_msg), "GetLastError() = %d", err);
      |                                                                    ~~   ^~~
      |                                                                    %lu
1 warning generated.
In file included from .\src\ninja.cc:21:
In file included from D:/Dev/llvm-mingw/include/c++/v1/algorithm:1842:
In file included from D:/Dev/llvm-mingw/include/c++/v1/__algorithm/for_each.h:16:
In file included from D:/Dev/llvm-mingw/include/c++/v1/__ranges/movable_box.h:21:
In file included from D:/Dev/llvm-mingw/include/c++/v1/optional:1294:
In file included from D:/Dev/llvm-mingw/include/c++/v1/memory:944:
In file included from D:/Dev/llvm-mingw/include/c++/v1/__memory/inout_ptr.h:16:
In file included from D:/Dev/llvm-mingw/include/c++/v1/__memory/shared_ptr.h:32:
D:/Dev/llvm-mingw/include/c++/v1/__memory/unique_ptr.h:80:5: warning: delete called on non-final 'ManifestParser' that has virtual functions but non-virtual destructor [-Wdelete-non-abstract-non-virtual-dtor]
   80 |     delete __ptr;
      |     ^
D:/Dev/llvm-mingw/include/c++/v1/__memory/unique_ptr.h:292:7: note: in instantiation of member function 'std::default_delete<ManifestParser>::operator()' requested here
  292 |       __ptr_.second()(__tmp);
      |       ^
D:/Dev/llvm-mingw/include/c++/v1/__memory/unique_ptr.h:261:71: note: in instantiation of member function 'std::unique_ptr<ManifestParser>::reset' requested here
  261 |   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() { reset(); }
      |                                                                       ^
.\src/manifest_parser.h:41:8: note: in instantiation of member function 'std::unique_ptr<ManifestParser>::~unique_ptr' requested here
   41 | struct ManifestParser : public Parser {
      |        ^
1 warning generated.
wrote build.ninja.
@jhasse
Copy link
Collaborator

jhasse commented Nov 30, 2024

These are only warnings, but still we should fix them. @sesse can you have a look?

We should also think about using -Werror on CI so that we don't merge warnings to the master branch in the future.

@sesse
Copy link

sesse commented Nov 30, 2024

The GetProcAddress() stuff is preexisting, right? There are the ones in hash_table8.hpp, but I guess they are just… bogus warnings that we should turn off?

@sesse
Copy link

sesse commented Nov 30, 2024

Something like this?

--- a/configure.py
+++ b/configure.py
@@ -340,6 +340,8 @@ if platform.is_msvc():
               '/wd4355',
               # Disable warnings about ignored typedef in DbgHelp.h
               '/wd4091',
+              # Disable warnings about redefined locals in hash_table8.hpp
+              '/wd4456',
               '/GR-',  # Disable RTTI.
               '/Zc:__cplusplus',
               # Disable size_t -> int truncation warning.

@zufuliu
Copy link
Contributor Author

zufuliu commented Nov 30, 2024

The shadow warning exists when using GCC with -Wshadow or -Wshadow=local:

H:\llvm\ninja>configure.py --bootstrap --platform mingw
bootstrapping ninja...
warning: A compatible version of re2c (>= 0.15.3) was not found; changes to src/*.in.cc will not affect your build.
In file included from .\src\hash_map.h:23,
                 from .\src\build_log.h:21,
                 from .\src\build.cc:31:
.\src\third_party/emhash/hash_table8.hpp: In member function 'emhash8::HashMap<KeyT, ValueT, HashT, EqT, Allocator, Policy>::size_type emhash8::HashMap<KeyT, ValueT, HashT, EqT, Allocator, Policy>::find_filled_bucket(const KeyT&, uint64_t) const':
.\src\third_party/emhash/hash_table8.hpp:1340:28: warning: declaration of 'slot' shadows a previous local [-Wshadow=compatible-local]
 1340 |                 const auto slot = _index[next_bucket].slot & _mask;
      |                            ^~~~
.\src\third_party/emhash/hash_table8.hpp:1329:20: note: shadowed declaration is here
 1329 |         const auto slot = _index[bucket].slot & _mask;
      |                    ^~~~
.\src\third_party/emhash/hash_table8.hpp: In member function 'emhash8::HashMap<KeyT, ValueT, HashT, EqT, Allocator, Policy>::size_type emhash8::HashMap<KeyT, ValueT, HashT, EqT, Allocator, Policy>::find_filled_slot(const K&) const':
.\src\third_party/emhash/hash_table8.hpp:1375:28: warning: declaration of 'slot' shadows a previous local [-Wshadow=compatible-local]
 1375 |                 const auto slot = _index[next_bucket].slot & _mask;
      |                            ^~~~
.\src\third_party/emhash/hash_table8.hpp:1364:20: note: shadowed declaration is here
 1364 |         const auto slot = _index[bucket].slot & _mask;
      |                    ^~~~

@zufuliu
Copy link
Contributor Author

zufuliu commented Nov 30, 2024

The GetProcAddress() stuff

These [-Wcast-function-type] warings can be fixed with std::bit_cast() or memcpy().

@zufuliu
Copy link
Contributor Author

zufuliu commented Dec 2, 2024

Reported these shadow warnings to upstream at ktprime/emhash#42

@ktprime
Copy link

ktprime commented Dec 3, 2024

Reported these shadow warnings to upstream at ktprime/emhash#42

thanks, I have fixed the issues in latest version.

@zufuliu
Copy link
Contributor Author

zufuliu commented Dec 15, 2024

All GCC and Clang warnings were fixed, can we update emhash/hash_table8.hpp to latest version to fix MSVC shadow warning?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants