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-----
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.