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ć

Attachment: signature.asc
Description: PGP signature

Reply via email to