Skip to content

Fix format conflict with std::format - #419

Merged
mtfishman merged 5 commits into
v3from
format_conflict
Jul 17, 2023
Merged

Fix format conflict with std::format#419
mtfishman merged 5 commits into
v3from
format_conflict

Conversation

@mtfishman

Copy link
Copy Markdown
Member

Trying to address #418.

The unfortunate part is that this relies on not introducing format from tinyformat into the namespace, so it would break user code that depends on just being able to directly use format.

I couldn't reproduce the original issue locally, @Wentzell could you see if this fixes the issue you saw? If it does then we can try to think of a safer fix.

@Wentzell

Copy link
Copy Markdown

With these changes I can successfully compile ITensor using gcc-13.
Thank you for looking into this so quickly @mtfishman

@mtfishman

Copy link
Copy Markdown
Member Author

@emstoudenmire any opinion on how to deal with the namespace clash of tinyformat::format, now that there is a std::format?

@mtfishman

Copy link
Copy Markdown
Member Author

@Wentzell glad that helped, I guess at least you and others can use this branch with the newer compilers while we decide on a more permanent solution.

@emstoudenmire

Copy link
Copy Markdown
Contributor

Thanks Matt, I think this is a good fix to just to not introduce format into the namespace. I think it's now better to leave it up to users whether they want to do something like using std::format; or using tfm::format; or just use the qualified names. Even though technically this is a breaking change, I think it's a very minor one in the sense of I wonder how many users were using format as introduced by ITensor?

@mtfishman

Copy link
Copy Markdown
Member Author

format is being used in some examples so I imagine some people copied it from there. I'm ok making this change, as you say it's easy enough to tell people to add using tfm::format; at the top of their code.

I guess with the new website we've lost our "News" section where we used to announce new versions and breaking changes for C++ ITensor, would be nice to find a place to announce this on the website so we don't catch people off guard. We can also announce things like that on the Discourse forum. For Julia we just do it in the Julia docs and Github README.

@emstoudenmire

emstoudenmire commented Jun 23, 2023 via email

Copy link
Copy Markdown
Contributor

@mtfishman

Copy link
Copy Markdown
Member Author

@emstoudenmire would you be ready to make a post about this if I merge it soon? @Wentzell wants this merged so they can use the official ITensor version in their CI tests.

@mtfishman

Copy link
Copy Markdown
Member Author

@emstoudenmire should I merge this?

@emstoudenmire

Copy link
Copy Markdown
Contributor

I've added a News entry publicly about this now on https://itensor.org

Please have a look to let me know if it's accurate.

@mtfishman

Copy link
Copy Markdown
Member Author

Thanks, looks great and that will be very helpful to have going forward! To be on the conservative side, I'm going to avoid introducing the tfm shorthand into the namespace by default, so users will have to use tinyformat::format. Could you change that "News" item appropriately? We could mention that the tfm shorthand can be used by defining:

namespace tfm = tinyformat;

@mtfishman
mtfishman merged commit 06c04b8 into v3 Jul 17, 2023
@mtfishman
mtfishman deleted the format_conflict branch July 17, 2023 14:29
@mtfishman

Copy link
Copy Markdown
Member Author

@emstoudenmire I updated the News item already.

@emstoudenmire

Copy link
Copy Markdown
Contributor

Thanks for updating it!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants