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

Remove non-compiling hash_value function #6533

Merged

Conversation

nirinchev
Copy link
Member

What, How & Why?

stdext::hash_value doesn't seem to exist on Windows and we already define a hashing function in

HASH_NAMESPACE_START
template<> struct hash<S2CellId> {
size_t operator()(S2CellId const& id) const {
return static_cast<size_t>(id.id() >> 32) + static_cast<size_t>(id.id());
}
};
HASH_NAMESPACE_END

Copy link
Contributor

@kiburtse kiburtse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird that it didn't trigger our CI, and also worked for me on windows. Could you post the error which you've hit? Just for reference. Function is indeed not needed. We may probably clean a few more of them.

@nirinchev
Copy link
Member Author

This is the incremental build output when I build from the feature/geo.. branch:

-- Selecting Windows SDK version 10.0.22621.0 to target Windows 10.0.
-- CMake version: 3.24.202208181-MSVC_2
Dependencies: PACKAGE_NAME=realm-core;VERSION=13.9.0;OPENSSL_VERSION=3.0.8;WIN32_ZLIB_VERSION=1.2.13;MDBREALM_TEST_SERVER_TAG=2023-03-16
-- Configuring done
-- Generating done
-- Build files have been written to: C:/work/dotnet/wrappers/cmake/Windows/Debug-ARM64
MSBuild version 17.4.0+18d5aef85 for .NET Framework
  Checking Build System
  Bid.vcxproj -> C:\work\dotnet\wrappers\cmake\Windows\Debug-ARM64\realm-core\src\external\IntelRDFPMathLib20U2\Bid.dir
  \Debug\Bid.lib
  s2cellid.cc
C:\work\dotnet\wrappers\realm-core\src\external\s2\s2cellid.cc(525,27): error C2039: 'hash_value': is not a member of 'stdext' [C:\work\dotnet\wrappers\cmake\Windows\Debug-ARM64\realm-core\src\external\s2\s2geometry.vcxproj]
C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.34.31933\include\iterator(1426,1): message : see declaration of 'stdext' [C:\work\dotnet\wrappers\cmake\Windows\Debug-ARM64\realm-core\src\external\s2\s2geometry.vcxproj]
C:\work\dotnet\wrappers\realm-core\src\external\s2\s2cellid.cc(525,37): error C2988: unrecognizable template declaration/definition [C:\work\dotnet\wrappers\cmake\Windows\Debug-ARM64\realm-core\src\external\s2\s2geometry.vcxproj]
C:\work\dotnet\wrappers\realm-core\src\external\s2\s2cellid.cc(525,37): error C2143: syntax error: missing ';' before '<' [C:\work\dotnet\wrappers\cmake\Windows\Debug-ARM64\realm-core\src\external\s2\s2geometry.vcxproj]
C:\work\dotnet\wrappers\realm-core\src\external\s2\s2cellid.cc(525,37): error C2059: syntax error: '<' [C:\work\dotnet\wrappers\cmake\Windows\Debug-ARM64\realm-core\src\external\s2\s2geometry.vcxproj]
C:\work\dotnet\wrappers\realm-core\src\external\s2\s2cellid.cc(525,68): error C2143: syntax error: missing ';' before '{' [C:\work\dotnet\wrappers\cmake\Windows\Debug-ARM64\realm-core\src\external\s2\s2geometry.vcxproj]
C:\work\dotnet\wrappers\realm-core\src\external\s2\s2cellid.cc(525,68): error C2447: '{': missing function header (old-style formal list?) [C:\work\dotnet\wrappers\cmake\Windows\Debug-ARM64\realm-core\src\external\s2\s2geometry.vcxproj]

Not sure if my MSBuild version is different from yours or if it's because I'm building this for Arm.

@nirinchev nirinchev merged commit eb37390 into feature/geospatial_queries_for_points Apr 25, 2023
5 of 7 checks passed
@nirinchev nirinchev deleted the ni/remove-windows-hash branch April 25, 2023 22:00
ironage added a commit that referenced this pull request May 8, 2023
* Add s2geometry sources for geo queries support from mongodb repo as is

* s2: cleanup/remove unneeded files

* s2: add to build

* Add basic test for s2geometry link

* Object store Geospatial mappings

* starting queries

* s2: cleanup tests related functions

* Fix build due to removed error categories

* Replace std exceptions with realm specific

* Move geo query tests to separate test file for future expansion

* Add init types for polygon and center sphere

* Add basic geo_within support for Polygon and CenterSphere

* Fix windows build

* Style: rename geo constants

* Fix MacOS compatibility build issue

* More win/mac build fixes

* Add s2geometry library for swift package build

* Suppress s2 warnings for gcc

* fix clang-format complaint

* Trim down unnecessary S2 sources (#6456)

* integrate Realm logging and assertions

* remove some string utilities

* remove encoder/decoder

* remove some base files

* remove hash

* fix header search path for s2 swift builds

* Fix include of external when built as subproject of sdk

* Use modern language features (#6466)

* scoped_ptr -> std::unique_ptr

* use std type_traits

* Replace deprecated is_pod with is_trivial

---------

Co-authored-by: Kirill Burtsev <kirill.burtsev@mongodb.com>

* Use radians for CenterSphere to be unit-neutral (#6496)

* Don't use optional for sphere radius to reduce the size of Geospatial

* Use radians for CenterSphere radius, expose constant

* Add comments about points in Box and Polygon

* Don't construct optional needlessly on Geospatial comparison

* Geospatial RQL (#6352)

* geospatial query parser support WIP

* some error handling

* RQL geoSphere

* RQL geoPolygon

* query geospatial arguments

* C-API geospatial in mixed

* add a Geospatial type checking API

* format

* fix warnings on Windows

* Alternative syntax definition

* API changes from feedback and testing

* remove type_GeoPoint as Geospatial is an umbrella for this

* additional tests

* fix an unused warning

* review feedback

* Use NaN for GeoPoint (#6490)

* Use NaN for altitude, expose to public

* Check for NaN for lat/lon

---------

Co-authored-by: James Stone <james.stone@mongodb.com>

* formatting

* remove GeospatialRef and use Geospatial* remove wrong C-API implementation (#6518)

---------

Co-authored-by: Jørgen Edelbo <jorgen.edelbo@mongodb.com>
Co-authored-by: Kirill Burtsev <kirill.burtsev@mongodb.com>

* Add REALM_ENABLE_GEOSPATIAL cmake option to disable feature (#6525)

* Allow to disable compilation of whole geospatial support
* Turn on the geo feature for swift build by default

* Remove non-compiling hash_value function (#6533)

* Geospatial Polygons support holes (#6529)

* Geospatial uses a variant polygon has holes

* support holes in polygons in RQL

* touch ups

* Add REALM_ENABLE_GEOSPATIAL to config.h

* add missing include

* Fix test build for geo feature, optimize copying (#6550)

* Fix build without geospatial

* Don't force to copy primitives on get<>

* Allow less curly braces for GeoPolygon on init

* format

---------

Co-authored-by: Jørgen Edelbo <jorgen.edelbo@mongodb.com>
Co-authored-by: Kirill Burtsev <kirill.burtsev@mongodb.com>

* changelog and test without geospatial on CI

* disable geospatial by default for cocoa builds, review feedback

* turn off geospatial in swift builds for now

* formatting

---------

Co-authored-by: Kirill Burtsev <kirill.burtsev@mongodb.com>
Co-authored-by: Jørgen Edelbo <jorgen.edelbo@mongodb.com>
Co-authored-by: Nikola Irinchev <irinchev@me.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants