Skip to content

Commit

Permalink
Added code to generate proper JS bindings of translator
Browse files Browse the repository at this point in the history 
 - COMPILE_WASM cmake option sets WASM_BINDINGS compile
   definition that enables code for generating proper JS
   bindings
  • Loading branch information
@abhi-agg
abhi-agg committed Feb 11, 2021
1 parent 23a9527 commit 7b80003
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
22 changes: 21 additions & 1 deletion src/TranslationResult.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ class TranslationResult {
public:
typedef std::vector<std::pair<std::string_view, std::string_view>>
SentenceMappings;

#ifdef WASM_BINDINGS
TranslationResult(const std::string &original, const std::string &translation)
: originalText(original), translatedText(translation),
sentenceMappings() {}
#endif
TranslationResult(const std::string &original, const std::string &translation,
SentenceMappings &sentenceMappings)
: originalText(original), translatedText(translation),
Expand All @@ -31,13 +35,29 @@ class TranslationResult {
translatedText(std::move(other.translatedText)),
sentenceMappings(std::move(other.sentenceMappings)) {}

#ifdef WASM_BINDINGS
TranslationResult(const TranslationResult &other)
: originalText(other.originalText),
translatedText(other.translatedText),
sentenceMappings(other.sentenceMappings) {}
#endif

TranslationResult(std::string &&original, std::string &&translation,
SentenceMappings &&sentenceMappings)
: originalText(std::move(original)),
translatedText(std::move(translation)),
sentenceMappings(std::move(sentenceMappings)) {}

#ifndef WASM_BINDINGS
TranslationResult &operator=(const TranslationResult &) = delete;
#else
TranslationResult &operator=(const TranslationResult &result) {
originalText = result.originalText;
translatedText = result.translatedText;
sentenceMappings = result.sentenceMappings;
return *this;
}

This comment has been minimized.

Sign in to view

Copy link
@jerinphilip

jerinphilip Feb 15, 2021 •
edited

Contributor

I'm not sure if there are other ways, but I think this is a terrible idea.

This creates a copy of result.originalText into this->originalText string_views inside this->sentenceMappings refer to result.originalText. You should be preventing the CopyAssignment wherever it is being called, this can cause a lot of problems, at least did to me. The deletion of the copy-constructor is so that when this happens it's pointed out during compile time. Adding it back with ifdefs is just setting up more trouble.
#endif

/* Return the original text. */
const std::string &getOriginalText() const { return originalText; }
Expand Down
2 changes: 2 additions & 0 deletions src/translator/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ endif()
if(COMPILE_WASM)
# A dirty hack because of marian's bad cmake practices
target_compile_definitions(bergamot-translator PUBLIC USE_SSE2 WASM)
# Enable code that is required for generating JS bindings
target_compile_definitions(bergamot-translator PRIVATE WASM_BINDINGS)
endif(COMPILE_WASM)

if (COMPILE_THREAD_VARIANT)
Expand Down

0 comments on commit 7b80003

Please sign in to comment.