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

baggage.h is missing #include <cctype> for MSVC2015 #808

Closed
lidavidm opened this issue May 28, 2021 · 3 comments · Fixed by #822
Closed

baggage.h is missing #include <cctype> for MSVC2015 #808

lidavidm opened this issue May 28, 2021 · 3 comments · Fixed by #822
Assignees
Labels
bug Something isn't working

Comments

@lidavidm
Copy link
Contributor

Describe your environment

Windows 10 x64
MSVC 19.00.24210 (VS2015)

Steps to reproduce

OpenTelemetry (latest main @ 12e56f9) was built with cmake -GNinja .. -DWITH_API_ONLY=ON -DCMAKE_INSTALL_PREFIX=../install -DBUILD_TESTING=OFF.

Then I tried to build a trivial application including an OpenTelemetry header:

#include <opentelemetry/baggage/baggage.h>

int main(int, char**) {
    return 0;
}

What is the expected behavior?
The application should build successfully.

What is the actual behavior?

C:\Users\User\opentelemetry-cpp\install>cl.exe -Iinclude /EHsc /DHAVE_ABSEIL_VARIANT=1 test.cc
Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24210 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

test.cc
include\opentelemetry/baggage/baggage.h(225): error C2039: 'isalnum': is not a member of 'std'
c:\users\user\opentelemetry-cpp\install\include\opentelemetry\nostd\./absl/types/variant.h(809): note: see declaration of 'std'
include\opentelemetry/baggage/baggage.h(248): error C2039: 'isdigit': is not a member of 'std'
c:\users\user\opentelemetry-cpp\install\include\opentelemetry\nostd\./absl/types/variant.h(809): note: see declaration of 'std'
include\opentelemetry/baggage/baggage.h(252): error C2039: 'isdigit': is not a member of 'std'
c:\users\user\opentelemetry-cpp\install\include\opentelemetry\nostd\./absl/types/variant.h(809): note: see declaration of 'std'
include\opentelemetry/baggage/baggage.h(252): error C2039: 'toupper': is not a member of 'std'
c:\users\user\opentelemetry-cpp\install\include\opentelemetry\nostd\./absl/types/variant.h(809): note: see declaration of 'std'
include\opentelemetry/baggage/baggage.h(273): error C2039: 'isalnum': is not a member of 'std'
c:\users\user\opentelemetry-cpp\install\include\opentelemetry\nostd\./absl/types/variant.h(809): note: see declaration of 'std'

Additional context

Adding #include <cctype> to baggage.h fixes the issue.

@lidavidm lidavidm added the bug Something isn't working label May 28, 2021
@ThomsonTan
Copy link
Contributor

@lidavidm thanks for reporting and investigation. Would you like to submit a PR with the fix?

@maxgolov
Copy link
Contributor

maxgolov commented May 29, 2021

If you don't mind, could you please apply the fix, and try pulling locally from this PR:
#771

This PR eliminates the HAVE_ABSEIL_VARIANT option. Making Abseil Variant now the default option. That way we can build with MSVC2015 and the binary is going to be compatible with 2017 and 2019 (and hopefully 2022). I think we also need to add unit tests for baggage - to catch issues like this. Surprised we didn't catch it in my other PR. I'm planning to add CI loop for vs2015 once the above PR is merged.

@lidavidm
Copy link
Contributor Author

lidavidm commented Jun 2, 2021

Sorry about the delay - I'll submit a PR and test against the Abseil branch when I get a chance.

lidavidm added a commit to lidavidm/opentelemetry-cpp that referenced this issue Jun 3, 2021
@maxgolov maxgolov mentioned this issue Jun 4, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants