Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/ir/names.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ inline Name getValidName(Name root,
if (check(root)) {
return root;
}
auto prefixed = std::string(root.str) + separator;
auto prefixed = std::string(root.view()) + separator;
Index num = hint;
while (1) {
auto name = prefixed + std::to_string(num);
Expand Down
3 changes: 1 addition & 2 deletions src/ir/possible-contents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1211,8 +1211,7 @@ struct InfoCollector
addRoot(curr, PossibleContents::exactType(curr->type));
}
void visitStringConst(StringConst* curr) {
addRoot(curr,
PossibleContents::literal(Literal(std::string(curr->string.str))));
addRoot(curr, PossibleContents::literal(Literal(curr->string.view())));
}
void visitStringMeasure(StringMeasure* curr) {
// TODO: optimize when possible
Expand Down
2 changes: 1 addition & 1 deletion src/parser/contexts.h
Original file line number Diff line number Diff line change
Expand Up @@ -1996,7 +1996,7 @@ struct ParseDefsCtx : TypeParserCtx<ParseDefsCtx>, AnnotationParserCtx {
void setSrcLoc(const std::vector<Annotation>& annotations) {
const Annotation* annotation = nullptr;
for (auto& a : annotations) {
if (a.kind.str == std::string_view("src")) {
if (a.kind.view() == std::string_view("src")) {
annotation = &a;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/passes/Asyncify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1730,7 +1730,7 @@ struct AsyncifyLocals : public WalkerPass<PostWalker<AsyncifyLocals>> {
} // anonymous namespace

static std::string getFullImportName(Name module, Name base) {
return std::string(module.str) + '.' + base.toString();
return module.toString() + '.' + base.toString();
}

struct Asyncify : public Pass {
Expand Down
2 changes: 1 addition & 1 deletion src/passes/J2CLOpts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ class ConstantHoister : public WalkerPass<PostWalker<ConstantHoister>> {
}

Name getEnclosingClass(Name name) {
return Name(name.str.substr(name.str.find_last_of('@')));
return Name(name.view().substr(name.view().find_last_of('@')));
}

AssignmentCountMap& assignmentCounts;
Expand Down
10 changes: 5 additions & 5 deletions src/passes/MinifyImportsAndExports.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ struct MinifyImportsAndExports : public Pass {
std::cout << ',';
}
std::cout << "\n [";
String::printEscaped(std::cout, key.first.str) << ", ";
String::printEscaped(std::cout, key.second.str) << ", ";
String::printEscaped(std::cout, new_.str) << "]";
String::printEscaped(std::cout, key.first.view()) << ", ";
String::printEscaped(std::cout, key.second.view()) << ", ";
String::printEscaped(std::cout, new_.view()) << "]";
}
}
std::cout << "\n ],\n\"exports\": [";
Expand All @@ -127,8 +127,8 @@ struct MinifyImportsAndExports : public Pass {
std::cout << ',';
}
std::cout << "\n [";
String::printEscaped(std::cout, key.second.str) << ", ";
String::printEscaped(std::cout, new_.str) << "]";
String::printEscaped(std::cout, key.second.view()) << ", ";
String::printEscaped(std::cout, new_.view()) << "]";
}
}
std::cout << "\n ]\n";
Expand Down
8 changes: 4 additions & 4 deletions src/passes/Print.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2566,7 +2566,7 @@ struct PrintExpressionContents
// Re-encode from WTF-16 to WTF-8.
std::stringstream wtf8;
[[maybe_unused]] bool valid =
String::convertWTF16ToWTF8(wtf8, curr->string.str);
String::convertWTF16ToWTF8(wtf8, curr->string.view());
assert(valid);
// TODO: Use wtf8.view() once we have C++20.
String::printEscaped(o, wtf8.str());
Expand Down Expand Up @@ -3190,7 +3190,7 @@ void PrintSExpression::visitExport(Export* curr) {
o << '(';
printMedium(o, "export ");
std::stringstream escaped;
String::printEscaped(escaped, curr->name.str);
String::printEscaped(escaped, curr->name.view());
printText(o, escaped.str(), false) << " (";
switch (curr->kind) {
case ExternalKind::Function:
Expand Down Expand Up @@ -3219,8 +3219,8 @@ void PrintSExpression::visitExport(Export* curr) {
void PrintSExpression::emitImportHeader(Importable* curr) {
printMedium(o, "import ");
std::stringstream escapedModule, escapedBase;
String::printEscaped(escapedModule, curr->module.str);
String::printEscaped(escapedBase, curr->base.str);
String::printEscaped(escapedModule, curr->module.view());
String::printEscaped(escapedBase, curr->base.view());
printText(o, escapedModule.str(), false) << ' ';
printText(o, escapedBase.str(), false) << ' ';
}
Expand Down
8 changes: 4 additions & 4 deletions src/passes/StringLifting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ struct StringLifting : public Pass {
// Encode from WTF-8 to WTF-16.
auto wtf8 = global->base;
std::stringstream wtf16;
bool valid = String::convertWTF8ToWTF16(wtf16, wtf8.str);
bool valid = String::convertWTF8ToWTF16(wtf16, wtf8.view());
if (!valid) {
Fatal() << "Bad string to lift: " << wtf8;
}
importedStrings[global->name] = wtf16.str();
importedStrings[global->name] = wtf16.view();
found = true;
}
}
Expand All @@ -101,7 +101,7 @@ struct StringLifting : public Pass {
continue;
}
// The index in the array is the basename.
Index index = std::stoi(std::string(global->base.str));
Index index = std::stoi(std::string(global->base.view()));
if (index >= array.size()) {
Fatal() << "StringLifting: bad index in string.const section";
}
Expand Down Expand Up @@ -222,7 +222,7 @@ struct StringLifting : public Pass {
auto iter = parent.importedStrings.find(curr->name);
if (iter != parent.importedStrings.end()) {
auto wtf16 = iter->second;
replaceCurrent(Builder(*getModule()).makeStringConst(wtf16.str));
replaceCurrent(Builder(*getModule()).makeStringConst(wtf16.view()));
modified = true;
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/passes/StringLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ struct StringGathering : public Pass {
// Re-encode from WTF-16 to WTF-8 to make the name easier to read.
std::stringstream wtf8;
[[maybe_unused]] bool valid =
String::convertWTF16ToWTF8(wtf8, string.str);
String::convertWTF16ToWTF8(wtf8, string.view());
assert(valid);
// Then escape it because identifiers must be valid UTF-8.
// TODO: Use wtf8.view() and escaped.view() once we have C++20.
Expand Down Expand Up @@ -246,7 +246,7 @@ struct StringLowering : public StringGathering {
if (auto* c = global->init->dynCast<StringConst>()) {
std::stringstream utf8;
if (useMagicImports &&
String::convertUTF16ToUTF8(utf8, c->string.str)) {
String::convertUTF16ToUTF8(utf8, c->string.view())) {
global->module = stringConstsModule;
global->base = Name(utf8.str());
} else {
Expand All @@ -263,7 +263,7 @@ struct StringLowering : public StringGathering {
} else {
json << ',';
}
String::printEscapedJSON(json, c->string.str);
String::printEscapedJSON(json, c->string.view());
jsonImportIndex++;
}
global->init = nullptr;
Expand Down
84 changes: 49 additions & 35 deletions src/support/istring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,43 @@
* limitations under the License.
*/

#include <limits>
#include <mutex>

#include "istring.h"
#include "mixed_arena.h"

namespace wasm {

std::string_view IString::interned(std::string_view s, bool reuse) {
// We need a set of string_views that can be modified in-place to minimize
// the number of lookups we do. Since set elements cannot normally be
// modified, wrap the string_views in a container that provides mutability
// even through a const reference.
struct MutStringView {
mutable std::string_view str;
MutStringView(std::string_view str) : str(str) {}
};
struct MutStringViewHash {
size_t operator()(const MutStringView& mut) const {
return std::hash<std::string_view>{}(mut.str);
const char* IString::interned(std::string_view s) {
if (s.data() == nullptr) {
return nullptr;
}
Comment on lines +26 to +28
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is is not valid for empty string views to have nullptr data pointers?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is valid, though we'd need to be careful with hashing etc. Anyhow, this empty string does not need any storage, so it seems simpler to just skip it (might also be faster but I didn't measure).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if someone tries to intern the string_view {nullptr, 0} and we create a View at nullptr, then trying to turn that View back into a string_view will segfault. That seems bad, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't a std::string_view be null (internal pointer is nullptr)? Where would it segfault?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where it tries to dereference the pointer in the View (actually 4 bytes before it) to get the size.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, View needs to special-case a null to avoid a read. I also see that passing nullptr, 0 is not defined behavior anyhow. Fixed.


// A set of interned Views, i.e., that contains our pascal-style strings. We
// need to query this using a std::string_view, as that is what we receive as
// input (turning it into pascal-style storage would add overhead). To do so,
// use overloading in the hash and equality functions (which works thanks to
// `is_transparent`).
struct InternedHash {
using is_transparent = void;
Comment thread
tlively marked this conversation as resolved.
size_t operator()(View v) const {
return std::hash<std::string_view>{}(v.view());
}
size_t operator()(std::string_view sv) const {
return std::hash<std::string_view>{}(sv);
}
};
struct MutStringViewEqual {
bool operator()(const MutStringView& a, const MutStringView& b) const {
return a.str == b.str;
struct InternedEqual {
using is_transparent = void;
bool operator()(View a, View b) const { return a.view() == b.view(); }
bool operator()(std::string_view a, View b) const { return a == b.view(); }
bool operator()(View a, std::string_view b) const { return a.view() == b; }
bool operator()(std::string_view a, std::string_view b) const {
return a == b;
}
};
using StringSet =
std::unordered_set<MutStringView, MutStringViewHash, MutStringViewEqual>;
using StringSet = std::unordered_set<View, InternedHash, InternedEqual>;

// The authoritative global set of interned string views.
static StringSet globalStrings;
Expand All @@ -54,34 +65,37 @@ std::string_view IString::interned(std::string_view s, bool reuse) {
// A thread-local cache of strings to reduce contention.
thread_local static StringSet localStrings;

auto [localIt, localInserted] = localStrings.insert(s);
if (!localInserted) {
Comment thread
tlively marked this conversation as resolved.
if (auto it = localStrings.find(s); it != localStrings.end()) {
// We already had a local copy of this string.
return localIt->str;
return it->internal;
}

// No copy yet in the local cache. Check the global cache.
std::unique_lock<std::mutex> lock(mutex);
auto [globalIt, globalInserted] = globalStrings.insert(s);
if (!globalInserted) {
if (auto it = globalStrings.find(s); it != globalStrings.end()) {
// We already had a global copy of this string. Cache it locally.
localIt->str = globalIt->str;
return localIt->str;
localStrings.insert(*it);
return it->internal;
}

if (!reuse) {
// We have a new string, but it doesn't have a stable address. Create a copy
// of the data at a stable address we can use. Make sure it is null
// terminated so legacy uses that get a C string still work.
char* data = (char*)arena.allocSpace(s.size() + 1, 1);
std::copy(s.begin(), s.end(), data);
data[s.size()] = '\0';
s = std::string_view(data, s.size());
}
// We have a new string. Create a copy of the data at a stable address with a
// header we can use. Make sure it is null terminated so legacy uses that get
// a C string still work.
size_t size = s.size();
// The string's size must fit in 32 bits.
assert(size <= std::numeric_limits<uint32_t>::max());
char* buffer =
(char*)arena.allocSpace(sizeof(uint32_t) + size + 1, alignof(uint32_t));
*(uint32_t*)(buffer) = size;
char* data = buffer + sizeof(uint32_t);
std::copy(s.begin(), s.end(), data);
data[size] = '\0';

// Intern our new string.
localIt->str = globalIt->str = s;
return s;
View v{data};
globalStrings.insert(v);
localStrings.insert(v);
return data;
}

} // namespace wasm
Loading
Loading