Re: [PATCH gcc/* 1/2] gcc: copy expected and optional out of rust/utils
Arsen Arsenović <aarsenovic-rdvid1DuHRBWk0Htik3J/[email protected]>
| Newsgroups | gmane.comp.gcc.jit,gmane.comp.gcc.patches,gmane.comp.gcc.fortran,gmane.comp.gcc.algol68,gmane.comp.gcc.rust |
|---|---|
| Organization | BayLibre |
| Message-ID | <[email protected]> |
Jason Merrill <[email protected]> writes: > On 7/5/26 5:15 PM, Arsen Arsenović wrote: >> In my attempts to remove these duplicates from the Rust frontend, I >> broke it horribly. So, I'm leaving them in the Rust frontend. Help >> wanted! >> gcc/ChangeLog: >> * util/expected.h: New file. Copied out of rust/util. > > Creating gcc/util seems novel, usually utility headers go directly in gcc/ or > in include/. Yes, that's the structure the Rust FE was using, and I though it'd be nice to curtail expanding gcc/ a bit (it's already at 1095 entries), so I kept it. But, I can move it elsewhere if so desired. What may be an issue with include/ is that (at the moment, anyway) the headers are using gcc_assert. > It might also be useful to put these in their own directory to make it > clearer that they're imported from the tl libraries, for ease of > updating. I suppose that doesn't need to be a top-level directory, > perhaps include/tl as in the original repo? Sure, whatever we land on can get an extra subdir. >> +// You should have received a copy of the CC0 Public Domain Dedication >> +// along with this software. If not, see >> +// <http://creativecommons.org/publicdomain/zero/1.0/>. > > I don't think there is such a copy in the GCC sources, is there? > > This wording seems copied from the GPL notice and much less relevant > to CC0, but it's awkward to have this unsatisfied "should". > > Putting these headers in their own directory will also make it simpler > to include such a file. This verbiage is actually in the original, ISTM: https://github.com/TartanLlama/expected/blob/master/include/tl/expected.hpp https://github.com/TartanLlama/optional/blob/master/include/tl/optional.hpp But, indeed, I do not see the string "CC0 1.0 Universal" in the gcc tree. >> +/* Includes removed, make sure to include system.h first. */ >> +#include "expected_fwd.h" > > Isn't it OK to keep the includes here as long as system.h gets in first? If this header contains includes, and someone forgets to use the appropriate #define INCLUDE_... before system.h, then a scenario is created where the poisons etc in system.h could happen not to break the build on the developers' machine, but break it on others' machines. If they're not included, then there's always a build error. Thus, it is easier to trip up over the poisons if the includes are also present in these headers. That said, a comment should probably say that one ought to INCLUDE_FUNCTIONAL before system.h and before including this header also, rather than leaving it to guesswork. > Are there other local edits to these files? Relative to the existing gcc/rust copies of these files, rust_assert was replaced with gcc_assert, but otherwise, no: $ git --no-pager diff -U0 typed-qualifiers:gcc/{rust/,}util/optional.h diff --git a/gcc/rust/util/optional.h b/gcc/util/optional.h index 9d2cd97cbedc..9808b199f35e 100644 --- a/gcc/rust/util/optional.h +++ b/gcc/util/optional.h @@ -20,0 +21,2 @@ +#include "optional_fwd.h" + @@ -25,2 +26,0 @@ -#include "rust-system.h" - @@ -1255 +1255 @@ public: - rust_assert(has_value ()); + gcc_assert(has_value ()); @@ -1265 +1265 @@ public: - rust_assert(has_value ()); + gcc_assert(has_value ()); @@ -1277 +1277 @@ public: - rust_assert(has_value ()); + gcc_assert(has_value ()); @@ -1288 +1288 @@ public: - rust_assert(has_value ()); + gcc_assert(has_value ()); @@ -1299 +1299 @@ public: - rust_assert(has_value ()); + gcc_assert(has_value ()); @@ -2033 +2033 @@ public: - rust_assert(has_value ()); + gcc_assert(has_value ()); @@ -2044 +2044 @@ public: - rust_assert(has_value ()); + gcc_assert(has_value ()); @@ -2055 +2055 @@ public: - rust_assert(has_value ()); + gcc_assert(has_value ()); @@ -2066 +2066 @@ public: - rust_assert(has_value ()); + gcc_assert(has_value ()); $ git --no-pager diff -U0 typed-qualifiers:gcc/{rust/,}util/expected.h diff --git a/gcc/rust/util/expected.h b/gcc/util/expected.h index 07ba8776214e..9a596dbe905e 100644 --- a/gcc/rust/util/expected.h +++ b/gcc/util/expected.h @@ -24 +24,2 @@ -#include "rust-system.h" +/* Includes removed, make sure to include system.h first. */ +#include "expected_fwd.h" Relative to upstream, I am not sure. The Rust FE maintainers would know. (Arthur?) -- Arsen Arsenović
signature.asc
(application/pgp-signature, 430 B)
-----BEGIN PGP SIGNATURE----- iQEKBAEWCgCyFiEE/uKz0RP8AKMWLWBhUsKUMB6ixJMFAmqEJ1kbFIAAAAAABAAO bWFudTIsMi41KzEuMTIsMiwyXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25z Lm9wZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRGRUUyQjNEMTEzRkMwMEEzMTYyRDYw NjE1MkMyOTQzMDFFQTJDNDkzGBxhYXJzZW5vdmljQGJheWxpYnJlLmNvbQAKCRBS wpQwHqLEk0YWAP9BlFcg9uo56r68m0Xkw6OmVY/MmssC8x487yjbHltgWgEAjsYV Rs6ayRt83pTTwu4XfUwTWZdn15DkkKCMtFvEewQ= =yMYj -----END PGP SIGNATURE-----