[Mojo-stdlib] Remove `FLAG_IS_STATIC_CONSTANT` from the `String` struct

[gabrieldemarmiesse]

While poking around in the String struct, I noticed that it was actually possible to know if we point to a StaticString without the need to read the flag FLAG_IS_STATIC_CONSTANT. We can use the condition "pointer is not null and capacity is 0". I also simplify the String() constructor in the process. Let's remove the flag and make it available for future use cases.

See https://github.com/modular/modular/blob/main/mojo/proposals/string-design.md for more info on the subject:

In addition to setting the bit to know whether the pointer is to a constant string, it is important to know that the String type also sets its notion of "capacity" to zero. This ensures that any attempt to append or access will immediately reallocate without extra checks. This is a minor optimization and simplification of code, but it means that static constant strings will have capacity=0 but have length=n where n > 0

@lattner expressed his desire to do this in this discord message

[JoeLoser]

!sync

[gabrieldemarmiesse]

@lattner I changed the string proposal document to reflect this change.

@JoeLoser I also addressed your comment in this PR. It's unreleated, so let me know if you want a separate PR instead for a cleaner git history.

[JoeLoser]

@lattner I changed the string proposal document to reflect this change.

@JoeLoser I also addressed your comment in this PR. It's unreleated, so let me know if you want a separate PR instead for a cleaner git history.

Let's tease that out to a separate PR that I'll sync and land immediately, please. I suspect this PR may run into a few test failures and I don't want to hold that changelog entry up with this.

[gabrieldemarmiesse]

@JoeLoser done :)

[JoeLoser]

@gabrieldemarmiesse looks like a few tests are failing as you can see in the Bazel builds (e.g. https://github.com/modular/modular/actions/runs/15196447847/job/42741618384?pr=4661). You can repro with ./bazelw test //mojo/stdlib/test:algorithm/test_elementwise.mojo.test for example. I haven't dug into them yet, but may be good to poke at them to understand how it's related to your changes. These tests are also run internally, so I can't land your PR internally until we fix them.

[gabrieldemarmiesse]

@JoeLoser I fixed the unit tests.

After a long investigation, I've put in evidence that there is currently (in main) either a UB in the stdlib, or a bug in the compiler causing a miscompilation. You can see the proof here and can play with the toy example:

• https://github.com/modular/modular/pull/4679

After finding the exact pattern which caused the error to trigger, it was quite easy to find a solution. Assigning local variables worked, but i've opt in to just do b and a instead of a and b. See this commit: 4217e64e04e855da233931bea4fb3b95d1ce49b4 which fixes the issue.

I'm not particularily eager to try to find the root of the UB or the miscompilation, so if someone else wants to do it, feel free to

[JoeLoser]

@JoeLoser I fixed the unit tests.

After a long investigation, I've put in evidence that there is currently (in main) either a UB in the stdlib, or a bug in the compiler causing a miscompilation. You can see the proof here and can play with the toy example:

• [Do not merge] Miscompilation or UB in String #4679

After finding the exact pattern which caused the error to trigger, it was quite easy to find a solution. Assigning local variables worked, but i've opt in to just do b and a instead of a and b. See this commit: 4217e64 which fixes the issue.

I'm not particularily eager to try to find the root of the UB or the miscompilation, so if someone else wants to do it, feel free to

Ooof, this looks like a fun one to track down. Awesome work chasing this down and fixing the unit tests @gabrieldemarmiesse! I'll mention this internally and someone will take a look; the community is also free to take a look (@soraros may be interested). I won't be able to take a look for a little while since I'm out on PTO next week.

I'll sync this in so we can land it 👍 🎉

[JoeLoser]

!sync

[soraros]

Ooof, this looks like a fun one to track down. Awesome work chasing this down and fixing the unit tests @gabrieldemarmiesse! I'll mention this internally and someone will take a look; the community is also free to take a look (@soraros may be interested). I won't be able to take a look for a little while since I'm out on PTO next week.

I will try to track down this UB. It's gonna be so fun. 🙃

[github-actions[bot]]

Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

---

I have read the CLA Document and I hereby sign the CLA

---

1 out of 2 committers have signed the CLA.
:white_check_mark: (gabrieldemarmiesse)[https://github.com/gabrieldemarmiesse]
:x: @lattner
_{You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.}

[gabrieldemarmiesse]

I have read the CLA Document and I hereby sign the CLA

[lattner]

@JoeLoser is there a path to getting this landed, Gabriel has been very patient here and it is a nice improvement 🙏

[JoeLoser]

!sync

[JoeLoser]

@JoeLoser is there a path to getting this landed, Gabriel has been very patient here and it is a nice improvement 🙏

Agreed! Sorry for the delay - I was on PTO and just came back today. Now that CI is passing, I've resynced this and it should land soon.

[modularbot]

✅🟣 This contribution has been merged 🟣✅

Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the main branch during the next Mojo nightly release, typically within the next 24-48 hours.

We use Copybara to merge external contributions, click here to learn more.

[modularbot]

Landed in a423ce152aca01d66c095204b7c4a147ad68c106! Thank you for your contribution 🎉