Fix class template translations#195
Conversation
|
|
||
| MyContainer<float> fmc; | ||
| assert(fmc.empty()); | ||
| fmc.push_back('a'); |
|
|
||
| std::string ToRustName(std::string name) { | ||
| size_t pos = 0; | ||
| std::string chars = "<>, "; |
There was a problem hiding this comment.
Maybe Mapper::ToString is a better place to do these replaces. We already have some similar replaces that we do to normalize the output
There was a problem hiding this comment.
ToString handles converting a given C++ construct to its corresponding string representation, whereas ToRustName is responsible for transforming the string representation of an arbitrary C++ construct into a valid Rust name. Given this distinction, wouldn't it be clearer to keep these two functions separate?
Additionally, this normalization is only required when we want to use the string representation of a given C++ construct as a Rust name. It is not required for the other uses of ToString, such as obtaining the string representation of a construct in order to look up or insert a translation rule.
| size_t pos = 0; | ||
| std::string chars = "<>, "; | ||
| while ((pos = name.find_first_of(chars, pos)) != std::string::npos) { | ||
| name.replace(pos, 1, 1, '_'); |
|
|
||
| std::string ToRustName(std::string name) { | ||
| size_t pos = 0; | ||
| std::string chars = "<>, "; |
There was a problem hiding this comment.
remove this var to avoid allocations (inline in find_first_of)
3 participants
Fixes how class templates are translated.
Class templates are currently monomorphized:
cpp2rust/cpp2rust/converter/converter.cpp
Lines 3316 to 3321 in 8f79a34
However, the name of each translated Rust struct was the plain C++ name, leading to collisions.
Additionally, the C++ name used in the translation rule inserted into Mapper was also the plain C++ name.
This name does not match how the type itself is printed, which led to internal cpp2rust assertion failures due to missing translation rules.