Skip to content

Conversation

@atta-ullah01
Copy link
Contributor

Replaces the FetchContent mechanism with a vendored copy of tl/expected.hpp (v1.1.0).

The header is placed in CGAL/tl/expected.hpp to avoid namespace pollution.
This also removes the CMake module CGAL_setup_tl-excepted.cmake.

Release Management

  • Affected package(s): Constrained_triangulation_3
  • Issue(s) solved (if any): CDT_3: ship <tl/expected.hpp> #9145
  • License and copyright ownership: CC0 1.0 Universal (tl/expected.hpp)

Replaces the FetchContent mechanism with a vendored copy of tl/expected.hpp (v1.1.0).

The header is placed in CGAL/tl/expected.hpp to avoid namespace pollution.
This also removes the CMake module CGAL_setup_tl-excepted.cmake.

## Release Management

* Affected package(s): Constrained_triangulation_3
* Issue(s) solved (if any): CGAL#9145
* License and copyright ownership: CC0 1.0 Universal (tl/expected.hpp)
@sloriot
Copy link
Member

sloriot commented Dec 19, 2025

Thanks for the PR. I'd move the tl/expected.hpp file into Installation package.

@lrineau lrineau linked an issue Dec 22, 2025 that may be closed by this pull request
@atta-ullah01
Copy link
Contributor Author

I added the SPDX tag to the header. I cannot run the detect_packages_licenses script locally without modifying unrelated files (version mismatch). Could you please update the license.txt manifest before merging?

@afabri
Copy link
Member

afabri commented Jan 6, 2026

@lrineau and @sloriot : Is there not a problem if someone already uses this header? Should "our" version go into a CGAL::tl namespace? Is it really needed that we use not yet standard language features?

@lrineau
Copy link
Member

lrineau commented Jan 12, 2026

@lrineau and @sloriot : Is there not a problem if someone already uses this header? Should "our" version go into a CGAL::tl namespace?

That is true. It could go in the CGAL::cpp23 namespace. I will make and test the change.

Is it really needed that we use not yet standard language features?

std::excepted was standardized in 2022, and part of C++23. That is a vocabulary type, like std::any, or std::optional. It is not really new. It can be seen as an improved version of std::optional: in case the object is void, an error object is in place, to describe why.

@afabri
Copy link
Member

afabri commented Jan 12, 2026

And does std::optional not offer enough ? It seems we add a lot of machinery for little.

- update to [version 1.3.1](https://github.com/TartanLlama/expected/releases/tag/v1.3.1)
- rename namespace `tl` to `CGAL::cpp23`
- rename the file to `<CGAL/expected.h>`

Once CGAL uses C++23. then that file will use `std::expected`.
@lrineau
Copy link
Member

lrineau commented Jan 12, 2026

And does std::optional not offer enough ? It seems we add a lot of machinery for little.

Obviously I agree. And the C++ committee as well. The "lot of machinery" is just one added file under licence CC0, after this PR.

@afabri
Copy link
Member

afabri commented Jan 12, 2026

And what about the define TL_... ? Will that not trigger already defined errors?

@lrineau
Copy link
Member

lrineau commented Jan 12, 2026

And what about the define TL_... ? Will that not trigger already defined errors?

I have renamed the macros in commit 0be2691.

And the new namespace is now CGAL::cpp23:: instead of tl:: (commit 7cf93c0).

@lrineau lrineau added this to the 6.2-beta milestone Jan 12, 2026
@afabri
Copy link
Member

afabri commented Jan 12, 2026

Let's also add it to the official documentation.

@sloriot
Copy link
Member

sloriot commented Jan 14, 2026

Successfully tested in CGAL-6.2-Ic-82

@sloriot sloriot merged commit 93093e8 into CGAL:main Jan 14, 2026
9 checks passed
@lrineau
Copy link
Member

lrineau commented Jan 15, 2026

Let's also add it to the official documentation.

That part was not yet done. There are probably other things added to STL_Extension that I had not yet documented.

@sloriot
Copy link
Member

sloriot commented Jan 15, 2026

you can still open an issue and doc it when ready. There is no need to delay the integration that can be the cause of a failure in the testsuite during the fetch content.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CDT_3: ship <tl/expected.hpp>

4 participants