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

Unknown type name 'uintptr_t' #1590

Closed
remia opened this issue Nov 8, 2023 · 7 comments
Closed

Unknown type name 'uintptr_t' #1590

remia opened this issue Nov 8, 2023 · 7 comments

Comments

@remia
Copy link
Contributor

remia commented Nov 8, 2023

We started seeing the following error pop up in OpenColorIO nightly build on ubuntu latest / C++ 20, for example. I believe it might be because uintptr_t is optional to support for C++. I'm not exactly sure what is the maximum size allowed to find a better way to derive, does it depends on the architecture like implied by this code?

/home/runner/work/OpenColorIO/OpenColorIO/_build/ext/build/openexr/src/openexr_install/src/bin/exrcheck/main.cpp:59:13: error: unknown type name 'uintptr_t'; did you mean 'intptr_t'?
      const uintptr_t kMaxSize = uintptr_t(-1) / 4;
            ^~~~~~~~~
            intptr_t
/usr/include/unistd.h:267:20: note: 'intptr_t' declared here
typedef __intptr_t intptr_t;
                   ^
/home/runner/work/OpenColorIO/OpenColorIO/_build/ext/build/openexr/src/openexr_install/src/bin/exrcheck/main.cpp:59:34: error: use of undeclared identifier 'uintptr_t'; did you mean 'intptr_t'?
      const uintptr_t kMaxSize = uintptr_t(-1) / 4;
                                 ^
/usr/include/unistd.h:267:20: note: 'intptr_t' declared here
typedef __intptr_t intptr_t;
                   ^
2 errors generated.
@meshula
Copy link
Contributor

meshula commented Nov 9, 2023

If std::numeric_limits<intptr_t>::max() works, the equivalent value could be generated via

std::numeric_limits<intptr_t>::max() / 2

Are you able to test that?

Since the /4 makes the number positive it doesn't seem important that kMaxSize needs to be a uintptr_t.

@cary-ilm
Copy link
Member

cary-ilm commented Nov 9, 2023

@reumia, that's curious as it's far from the only use of uintptr_t, it's used in the OpenEXR and OpenEXRCore libraries, so I would expect the failure there first if it's not defined, is that not happening? Are we missing an include?

@peterhillman, could you possibly take a look at this? The code in question declares kMaxSize as uintptr_t then only uses it on the following line where it's cast to streampos. Wouldn't it be better to simply declare it as streampos in the first place? And I'm not sure what edge case the uintptr_t(-1) is intended to catch, can that be simplified?

@peterhillman
Copy link
Contributor

I think @kdt3rd wrote that bit. It certainly looks cleverer than anything I'd write. I would have been tempted to put a constant in there for the maximum file size that will be loaded into memory. I think declaring it as streampos isn't equivalent: a processor could have a 32 bit memory pointer size but still support 64 bit file pointers.

@remia
Copy link
Contributor Author

remia commented Nov 9, 2023

Thanks for the answers all, this got me into looking a bit closer at the issue and while I don't understand everything yet, I believe this might be partly an issue coming from OpenColorIO, here are some more information:

  • We use OPENEXR_INSTALL_TOOLS instead of the correct OPENEXR_BUILD_TOOLS=OFF so in theory should not even build the tools and would not have seen any issues,
  • OpenColorIO platform latest nightly build try to use C++20, this might not even be supported / tested by OpenEXR (goal of this workflow is to find issues early though)
  • OpenColorIO still installs an "old" version of OpenEXR for this CI workflow, 3.1.5 by default (Apr. 2022)
  • Looking at the compilation log, OpenEXRCore compile flags do not include any -std=c++20 or -std=gnu++20, rest of the libraries (Iex, IlmThread) and tools do which probably trigger the error here (as @cary-ilm rightfully pointed out, OpenEXRCore use that type in a few places already). I don't know exactly why OpenEXRCore doesn't get the C++ standard flag but suspect this is related to it's dependency with Imath, will have to dig further to understand.

@cary-ilm
Copy link
Member

Following up on this, did this get resolved?

And @remia, in case it matters, the reason that OpenEXRCore doesn't get the C++ standard flag is that it's written in C, not C++.

@remia
Copy link
Contributor Author

remia commented Jan 15, 2024

Sorry for the silence @cary-ilm, I think the issue has more to do with the ubuntu-latest runners from Github than OpenEXR. After disabling OpenEXR's tools building, I got a new error this time in OCIO itself which seem to be more related to the STL than OCIO.

Quick internet search today yielded this issue GHA, I have to admit I didn't read all the comments in here but it appears to be affecting mostly C++20 builds (which this OCIO bleeding edge workflow uses) and is still not fixed.

Feel free to close this ticket!

@meshula
Copy link
Contributor

meshula commented Jan 15, 2024

Thanks for the follow up. Hopefully actions gets its configuration in order soon.

@meshula meshula closed this as completed Jan 15, 2024
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

No branches or pull requests

4 participants