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

Idea: use constexpr for int version functions #3222

Open
ankane opened this issue Apr 5, 2022 · 4 comments
Open

Idea: use constexpr for int version functions #3222

ankane opened this issue Apr 5, 2022 · 4 comments
Assignees
Labels
Feature Request Missing Feature/Wrapper Lang: C++ Native implementation issue
Milestone

Comments

@ankane
Copy link

ankane commented Apr 5, 2022

What language and solver does this apply to? C++

Describe the problem you are trying to solve.

Hi, I'd like to check the OR-Tools version during compilation to make sure it's compatible with the C++ code that uses it.

Describe the solution you'd like

Use constexpr for int version functions

constexpr int OrToolsMajorVersion();

constexpr int OrToolsMinorVersion();

constexpr int OrToolsPatchVersion();

so users can do

static_assert(
  operations_research::OrToolsMajorVersion() == 9 &&
  operations_research::OrToolsMinorVersion() == 3,
  "Incompatible OR-Tools version"
);

Describe alternatives you've considered

Another option would be to export the OR_TOOLS_MAJOR, OR_TOOLS_MINOR, and OR_TOOLS_PATCH constants.

@Mizux Mizux self-assigned this Apr 5, 2022
@Mizux Mizux added Feature Request Missing Feature/Wrapper Lang: C++ Native implementation issue labels Apr 5, 2022
@Mizux Mizux added this to To do in ToDo via automation Apr 5, 2022
@Mizux Mizux added this to the v9.4 milestone Apr 5, 2022
@lperron
Copy link
Collaborator

lperron commented Apr 6, 2022

not trivial.

constexpr int OrToolsMajorVersion() in the .h, defined in the .cc does not work.
in C++ 17, constexpr implies inline.

This means that the OR_TOOLS_VERSION must be used in the .h, which will not work as this will not be defined in users makefile including ortools/base/version.h

@Mizux
Copy link
Collaborator

Mizux commented Apr 6, 2022

I would like to use #cmakedefine since it would exactly fix this issue BUT we also need to support the bazel build -_-;
ref: https://cmake.org/cmake/help/latest/command/configure_file.html#example

@Mizux Mizux closed this as completed Apr 6, 2022
ToDo automation moved this from To do to Done Apr 6, 2022
@Mizux Mizux reopened this Apr 6, 2022
ToDo automation moved this from Done to In progress Apr 6, 2022
@ankane
Copy link
Author

ankane commented Apr 6, 2022

Thanks for the responses. If it's non-trivial, no worries.

fwiw, it looks like PyTorch uses a separate script with a note about Bazel that populates version.h.in.

EDIT(mizux): and the bazel wrapper around the python script:
pytorch/pytorch:BUILD.bazel#L1625-L1637

@Mizux Mizux modified the milestones: v9.4, v9.5 May 16, 2022
@Mizux Mizux moved this from In progress to Stall in ToDo May 20, 2022
@Mizux Mizux moved this from Stall to To do in ToDo May 20, 2022
@Mizux Mizux modified the milestones: v9.5, v9.6 Sep 29, 2022
@Mizux Mizux modified the milestones: v9.6, v9.7 Feb 3, 2023
@Mizux Mizux modified the milestones: v9.7, v9.8 Jul 28, 2023
@Mizux Mizux moved this from To do to Stall in ToDo Jul 28, 2023
@Mizux Mizux modified the milestones: v9.8, v9.9 Oct 13, 2023
@Mizux Mizux modified the milestones: v9.9, v10.0 Feb 14, 2024
@Mizux Mizux modified the milestones: v10.0, Backlog Apr 26, 2024
@Mizux Mizux removed this from Stall in ToDo Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Missing Feature/Wrapper Lang: C++ Native implementation issue
3 participants