Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concatenating strings in a more portable way -- for some unclear reason #1743

Closed
wants to merge 5 commits into from
Closed

Concatenating strings in a more portable way -- for some unclear reason #1743

wants to merge 5 commits into from

Conversation

massimiliano-leoni
Copy link
Contributor

Normal string concatenation was observed to fail on some machines for unclear reasons. This implementation seems to be more robust.

@jhale
Copy link
Member

jhale commented Oct 7, 2021

This is unexpected! It would be nice to have a bit of clarity on why this happens before continuing though. What C++ standard library version are you building against?

@michalhabera
Copy link
Contributor

This depends on the filesystem library which provides the filename string. It would be good to first understand the issue more. We could just use C++17 stdlib filesystem then.

@massimiliano-leoni
Copy link
Contributor Author

This is unexpected! It would be nice to have a bit of clarity on why this happens before continuing though. What C++ standard library version are you building against?

On this machine I believe I'm using libstdc++.so.6.0.28, provided by GCC 10.2.0.

@garth-wells
Copy link
Member

I don't think we can merge this without a clear understanding of what the cause is, and with very clear comments on it. The current code look perfectly compliant to me.

@garth-wells
Copy link
Member

@massimiliano-leoni Does https://stackoverflow.com/a/59823692 help? (adding s)

@massimiliano-leoni
Copy link
Contributor Author

@garth-wells I tried but I couldn't get it to compile, I'm getting some error I don't fully understand.

@garth-wells
Copy link
Member

Not reproducible, so closing.

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.

5 participants