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

Statically enforce some metrics properties at compile time. #716

Merged
merged 1 commit into from
Sep 26, 2022

Conversation

BillyONeal
Copy link
Member

This exposes that the lists of all metrics are actually std::arrays, which allows for compile time enforcement of properties like:

  • All metrics have entries in the name list
  • COUNT is the last entry in the enum list
  • All string metrics have corresponding example values

This exposes that the lists of all metrics are actually std::arrays, which allows for compile time enforcement of properties like:

* All metrics have entries in the name list
* COUNT is the last entry in the enum list
* All string metrics have corresponding example values
@BillyONeal BillyONeal merged commit 7c67433 into microsoft:main Sep 26, 2022
@BillyONeal BillyONeal deleted the combine-metrics branch September 26, 2022 18:44
@wolfv
Copy link
Contributor

wolfv commented Oct 12, 2022

It seems that this gives a compiler error on potentially older MSVC?
You can find the log from conda-forge here: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=584303&view=logs&jobId=a70f640f-cc53-5cd3-6cdc-236a1aa90802&j=a70f640f-cc53-5cd3-6cdc-236a1aa90802&t=f5d15007-a01c-5ad8-c9ce-4d519d3b275f

C:\PROGRA~2\MICROS~2\2019\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe  /nologo /TP -DVCPKG_BASE_VERSION=2999-12-31 -DVCPKG_VERSION=unknownhash -I%SRC_DIR%\include -I%SRC_DIR%\build\_deps\fmt-src\include /DWIN32 /D_WINDOWS /GR /EHsc /EHsc /Zi /guard:cf /O2 /Ob2 /DNDEBUG -MT -FC -permissive- -utf-8 /Yupch.h /FIpch.h /Zm200 -std:c++17 /showIncludes /FoCMakeFiles\vcpkglib.dir\src\vcpkg\metrics.cpp.obj /FdCMakeFiles\vcpkglib.dir\ /FS -c %SRC_DIR%\src\vcpkg\metrics.cpp
%SRC_DIR%\src\vcpkg\metrics.cpp(43): error C2370: 'vcpkg::all_define_metrics': redefinition; different storage class
%SRC_DIR%\include\vcpkg\metrics.h(49): note: see declaration of 'vcpkg::all_define_metrics'
%SRC_DIR%\src\vcpkg\metrics.cpp(71): error C2370: 'vcpkg::all_string_metrics': redefinition; different storage class
%SRC_DIR%\include\vcpkg\metrics.h(77): note: see declaration of 'vcpkg::all_string_metrics'
%SRC_DIR%\src\vcpkg\metrics.cpp(90): error C2370: 'vcpkg::all_bool_metrics': redefinition; different storage class
%SRC_DIR%\include\vcpkg\metrics.h(93): note: see declaration of 'vcpkg::all_bool_metrics'
[94/162] Building CXX object CMakeFiles\vcpkglib.dir\src\vcpkg\packagespec.cpp.obj
[95/162] Building CXX object CMakeFiles\vcpkglib.dir\src\vcpkg\install.cpp.obj

@BillyONeal
Copy link
Member Author

It seems that this gives a compiler error on potentially older MSVC?

We care about older Linux compilers because we know there are platforms we can't reach effectively (raspberrypi anyone?) but I don't see much reason to care about old msvc when we ship binaries that work on all Windowses.

@wolfv
Copy link
Contributor

wolfv commented Oct 14, 2022

@BillyONeal conda-forge works very much like a linux-distribution (but for Windows and macOS, too) and we're not that free to choose any random compiler :) I can try to update the compiler to e.g. VS2022 but still, I think some backwards compatibility and support for alternative package managers that package your software would be nice!

@BillyONeal
Copy link
Member Author

Can it just download our official copies when they match rather than rebuilding? We would take contributions to fix things but we aren't going to add regular testing for Windows compilers other than our release one.

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

Successfully merging this pull request may close these issues.

3 participants