From d1b3f03e2c02f9edf70d153fec91fcc58da0c962 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 2 Dec 2020 20:06:02 -0600 Subject: [PATCH 01/40] Add clang-tidy --- .clang-tidy | 4 ++++ package.json | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 .clang-tidy diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 00000000..2e8fb22b --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,4 @@ +--- +Checks: "*, -clang-diagnostic-*-compat, -cppcoreguidelines-init-variables, -modernize-return-braced-init-list, -misc-unused-parameters, -misc-non-private-member-variables-in-classes, -llvmlibc-*, -llvm-header-guard, -llvm-include-order, -modernize-use-trailing-return-type, -readability-avoid-const-params-in-decls, -readability-convert-member-functions-to-static, -fuchsia-default-arguments-declarations, -fuchsia-default-arguments-calls, -*-uppercase-literal-suffix, -fuchsia-overloaded-operator, -google-build-using-namespace, -google-global-names-in-headers, -google-readability-todo, -*-else-after-return, -*-braces-around-statements" +HeaderFilterRegex: ".*" +FormatStyle: none diff --git a/package.json b/package.json index b514e5c6..949b1403 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,9 @@ "test": "npm run test:node && npm run test:browser", "benchmark": "node benchmark/marker-index.benchmark.js", "prepublishOnly": "npm run build:browser", - "standard": "standard --recursive src test" + "standard": "standard --recursive src test", + "tidy": "clang-tidy src/core/*.cc src/core/*.h src/bindings/*.cc src/bindings/*.h src/bindings/em/*.cc src/bindings/em/*.h", + "tidy:fix": "npm run tidy -- --fix --fix-errors" }, "repository": { "type": "git", From c445df81285bd34a5c7aa415d644e7d61282c41f Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 2 Dec 2020 20:09:26 -0600 Subject: [PATCH 02/40] Add clang-format script --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 949b1403..9187b603 100644 --- a/package.json +++ b/package.json @@ -15,6 +15,7 @@ "benchmark": "node benchmark/marker-index.benchmark.js", "prepublishOnly": "npm run build:browser", "standard": "standard --recursive src test", + "format": "clang-format -i src/core/*.cc src/core/*.h src/bindings/*.cc src/bindings/*.h src/bindings/em/*.cc src/bindings/em/*.h", "tidy": "clang-tidy src/core/*.cc src/core/*.h src/bindings/*.cc src/bindings/*.h src/bindings/em/*.cc src/bindings/em/*.h", "tidy:fix": "npm run tidy -- --fix --fix-errors" }, From ef7a5ba6e9ddb58e534f562a953ac497c7e2b83b Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 04:16:48 -0600 Subject: [PATCH 03/40] Add standard variable + use c++14 --- binding.gyp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/binding.gyp b/binding.gyp index 249e9e82..c096277b 100644 --- a/binding.gyp +++ b/binding.gyp @@ -67,7 +67,8 @@ ], "variables": { - "tests": 0 + "tests": 0, + "STANDARD": 14, }, "conditions": [ @@ -111,9 +112,8 @@ }] }] ], - "target_defaults": { - "cflags_cc": ["-std=c++11"], + "cflags_cc": [ "-std=c++<(STANDARD)" ], "conditions": [ ['OS=="mac"', { "xcode_settings": { From a8296f11198ef0685ed887e4850d46ca7c4abb81 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 04:17:10 -0600 Subject: [PATCH 04/40] Add MACOSX_DEPLOYMENT_TARGET variable --- binding.gyp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/binding.gyp b/binding.gyp index c096277b..7134ff69 100644 --- a/binding.gyp +++ b/binding.gyp @@ -69,6 +69,7 @@ "variables": { "tests": 0, "STANDARD": 14, + "MACOSX_DEPLOYMENT_TARGET": "10.8" }, "conditions": [ @@ -101,11 +102,11 @@ "conditions": [ ['OS=="mac"', { 'cflags': [ - '-mmacosx-version-min=10.8' + "-mmacosx-version-min=<(MACOSX_DEPLOYMENT_TARGET)" ], "xcode_settings": { "GCC_ENABLE_CPP_EXCEPTIONS": "YES", - 'MACOSX_DEPLOYMENT_TARGET': '10.8', + 'MACOSX_DEPLOYMENT_TARGET': '<(MACOSX_DEPLOYMENT_TARGET)', } }] ] From d3f3d4038d89f9168a0cbfc8fabdf52703127742 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 04:19:48 -0600 Subject: [PATCH 05/40] Add Release configurations and optimizations --- binding.gyp | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/binding.gyp b/binding.gyp index 7134ff69..d2463875 100644 --- a/binding.gyp +++ b/binding.gyp @@ -130,6 +130,40 @@ "NOMINMAX" ], }] - ] - } + ], + 'default_configuration': 'Release', + 'configurations': { + # Release Settings + 'Release': { + 'defines': [ 'NDEBUG' ], + "cflags": [ "-fno-exceptions", "-Ofast" ], + "cflags_cc": [ "-fno-exceptions", "-Ofast", "-std=c++<(STANDARD)" ], + "xcode_settings": { + 'GCC_OPTIMIZATION_LEVEL': '3', # stop gyp from defaulting to -Os + "CLANG_CXX_LIBRARY": "libc++", + "CLANG_CXX_LANGUAGE_STANDARD": "c++<(STANDARD)", + 'MACOSX_DEPLOYMENT_TARGET': "<(MACOSX_DEPLOYMENT_TARGET)" + }, # XCODE + "msvs_settings": { + "VCCLCompilerTool": { + 'ExceptionHandling': 0, # /EHsc + 'MultiProcessorCompilation': 'true', + 'RuntimeTypeInfo': 'false', + 'Optimization': 3, # full optimizations /O2 == /Og /Oi /Ot /Oy /Ob2 /GF /Gy + 'StringPooling': 'true', # pool string literals + "AdditionalOptions": [ + # C++ standard + "/std:c++<(STANDARD)", + + # Optimizations + "/O2", + # "/Ob3", # aggressive inline + "/GL", # whole Program Optimization # /LTCG is implied with /GL. + "/DNDEBUG" # turn off asserts + ], + } + } # MSVC + }, # Release + }, # configurations + } # target-defaults } From 490e7856f05834e2bcd84bf45ff7b861776c20d6 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 04:29:56 -0600 Subject: [PATCH 06/40] Use C++17 standard --- binding.gyp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/binding.gyp b/binding.gyp index d2463875..85981bbb 100644 --- a/binding.gyp +++ b/binding.gyp @@ -68,7 +68,7 @@ "variables": { "tests": 0, - "STANDARD": 14, + "STANDARD": 17, "MACOSX_DEPLOYMENT_TARGET": "10.8" }, From 18aac6514b8897636da3576d97594cb01e26afed Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 04:40:36 -0600 Subject: [PATCH 07/40] Use Visual Studio 17 on AppVeyor --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index aa43e41e..132e2c69 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,4 +1,4 @@ -image: Visual Studio 2015 +image: Visual Studio 2017 environment: matrix: From ea704b2b183711ee4d518f934b0d9e0df8110d33 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 05:12:05 -0600 Subject: [PATCH 08/40] use native optional --- src/core/optional.h | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/src/core/optional.h b/src/core/optional.h index 61280d99..33852e05 100644 --- a/src/core/optional.h +++ b/src/core/optional.h @@ -1,29 +1,7 @@ #ifndef SUPERSTRING_OPTIONAL_H #define SUPERSTRING_OPTIONAL_H -#include +#include +using std::optional; -template class optional { - T value; - bool is_some; - -public: - optional(T &&value) : value(std::move(value)), is_some(true) {} - optional(const T &value) : value(value), is_some(true) {} - optional() : value(T()), is_some(false) {} - - T &operator*() { return value; } - const T &operator*() const { return value; } - const T *operator->() const { return &value; } - T *operator->() { return &value; } - operator bool() const { return is_some; } - bool operator==(const optional &other) { - if (is_some) { - return other.is_some && value == other.value; - } else { - return !other.is_some; - } - } -}; - -#endif // SUPERSTRING_OPTIONAL_H +#endif // SUPERSTRING_OPTIONAL_H From c787367758dd1b11c2db2d8e341bdbe8c722db5e Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 05:25:43 -0600 Subject: [PATCH 09/40] use constexpr instead of static const constexpr variable is guaranteed to have a value available at compile time. whereas static const members or const variable could either mean a compile time value or a runtime value. --- src/bindings/patch-wrapper.cc | 2 +- src/bindings/text-buffer-wrapper.cc | 2 +- src/core/encoding-conversion.cc | 8 ++++---- src/core/patch.cc | 2 +- src/core/text-buffer.cc | 10 +++++----- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/bindings/patch-wrapper.cc b/src/bindings/patch-wrapper.cc index fc610aaf..1cbf7fe2 100644 --- a/src/bindings/patch-wrapper.cc +++ b/src/bindings/patch-wrapper.cc @@ -17,7 +17,7 @@ static Nan::Persistent change_wrapper_constructor; static Nan::Persistent patch_wrapper_constructor_template; static Nan::Persistent patch_wrapper_constructor; -static const char *InvalidSpliceMessage = "Patch does not apply"; +constexpr char *InvalidSpliceMessage = "Patch does not apply"; class ChangeWrapper : public Nan::ObjectWrap { public: diff --git a/src/bindings/text-buffer-wrapper.cc b/src/bindings/text-buffer-wrapper.cc index 52e7bded..f84cf879 100644 --- a/src/bindings/text-buffer-wrapper.cc +++ b/src/bindings/text-buffer-wrapper.cc @@ -665,7 +665,7 @@ void TextBufferWrapper::is_modified(const Nan::FunctionCallbackInfo &info info.GetReturnValue().Set(Nan::New(text_buffer.is_modified())); } -static const int INVALID_ENCODING = -1; +constexpr int INVALID_ENCODING = -1; struct Error { int number; diff --git a/src/core/encoding-conversion.cc b/src/core/encoding-conversion.cc index 7259d40d..c1e64062 100644 --- a/src/core/encoding-conversion.cc +++ b/src/core/encoding-conversion.cc @@ -7,10 +7,10 @@ using std::function; using std::u16string; using std::vector; -static const uint32_t bytes_per_character = (sizeof(uint16_t) / sizeof(char)); -static const uint16_t replacement_character = 0xFFFD; -static const size_t conversion_failure = static_cast(-1); -static const float buffer_growth_factor = 2; +constexpr uint32_t bytes_per_character = (sizeof(uint16_t) / sizeof(char)); +constexpr uint16_t replacement_character = 0xFFFD; +constexpr size_t conversion_failure = static_cast(-1); +constexpr float buffer_growth_factor = 2; enum Mode { GENERAL, diff --git a/src/core/patch.cc b/src/core/patch.cc index 8702f9b3..ace85400 100644 --- a/src/core/patch.cc +++ b/src/core/patch.cc @@ -17,7 +17,7 @@ using std::ostream; using std::endl; using Change = Patch::Change; -static const uint32_t SERIALIZATION_VERSION = 1; +constexpr uint32_t SERIALIZATION_VERSION = 1; struct Patch::Node { Node *left; diff --git a/src/core/text-buffer.cc b/src/core/text-buffer.cc index c4dd3e8c..c36d4f1f 100644 --- a/src/core/text-buffer.cc +++ b/src/core/text-buffer.cc @@ -523,11 +523,11 @@ struct TextBuffer::Layer { // Next, calculate a score for each word indicating the quality of the // match against the query. - static const unsigned consecutive_bonus = 5; - static const unsigned subword_start_with_case_match_bonus = 10; - static const unsigned subword_start_with_case_mismatch_bonus = 9; - static const unsigned mismatch_penalty = 1; - static const unsigned leading_mismatch_penalty = 3; + constexpr unsigned consecutive_bonus = 5; + constexpr unsigned subword_start_with_case_match_bonus = 10; + constexpr unsigned subword_start_with_case_mismatch_bonus = 9; + constexpr unsigned mismatch_penalty = 1; + constexpr unsigned leading_mismatch_penalty = 3; vector matches; From 09d34bdfe28cdcaaaa5090a16875a5d957ee4c05 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 05:49:35 -0600 Subject: [PATCH 10/40] Exclude reserved identifiers from clang-tidy The codebase uses variables that are called `_edit`, `_ses`, etc. We should fix these later --- .clang-tidy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index 2e8fb22b..ae1f79b7 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,4 +1,4 @@ --- -Checks: "*, -clang-diagnostic-*-compat, -cppcoreguidelines-init-variables, -modernize-return-braced-init-list, -misc-unused-parameters, -misc-non-private-member-variables-in-classes, -llvmlibc-*, -llvm-header-guard, -llvm-include-order, -modernize-use-trailing-return-type, -readability-avoid-const-params-in-decls, -readability-convert-member-functions-to-static, -fuchsia-default-arguments-declarations, -fuchsia-default-arguments-calls, -*-uppercase-literal-suffix, -fuchsia-overloaded-operator, -google-build-using-namespace, -google-global-names-in-headers, -google-readability-todo, -*-else-after-return, -*-braces-around-statements" +Checks: "*, -clang-diagnostic-*-compat, -cppcoreguidelines-init-variables, -modernize-return-braced-init-list, -misc-unused-parameters, -misc-non-private-member-variables-in-classes, -llvmlibc-*, -llvm-header-guard, -llvm-include-order, -modernize-use-trailing-return-type, -readability-avoid-const-params-in-decls, -readability-convert-member-functions-to-static, -fuchsia-default-arguments-declarations, -fuchsia-default-arguments-calls, -*-uppercase-literal-suffix, -fuchsia-overloaded-operator, -google-build-using-namespace, -google-global-names-in-headers, -google-readability-todo, -*-else-after-return, -*-braces-around-statements, -bugprone-reserved-identifier, -cert-dcl37-c, -cert-dcl51-cpp" HeaderFilterRegex: ".*" FormatStyle: none From c5530089b6dc8c316c0205f8543561309f19152d Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 06:02:35 -0600 Subject: [PATCH 11/40] remove optional.h and include optional directly --- src/bindings/em/auto-wrap.h | 3 ++- src/bindings/marker-index-wrapper.cc | 3 ++- src/bindings/marker-index-wrapper.h | 3 ++- src/bindings/number-conversion.h | 3 ++- src/bindings/point-wrapper.h | 3 ++- src/bindings/range-wrapper.h | 3 ++- src/bindings/string-conversion.h | 3 ++- src/core/encoding-conversion.h | 3 ++- src/core/optional.h | 7 ------- src/core/patch.cc | 3 ++- src/core/patch.h | 3 ++- src/core/regex.h | 3 ++- src/core/text.h | 3 ++- 13 files changed, 24 insertions(+), 19 deletions(-) delete mode 100644 src/core/optional.h diff --git a/src/bindings/em/auto-wrap.h b/src/bindings/em/auto-wrap.h index ff4ff070..7fd3f938 100644 --- a/src/bindings/em/auto-wrap.h +++ b/src/bindings/em/auto-wrap.h @@ -7,7 +7,8 @@ #include #include "flat_set.h" #include "marker-index.h" -#include "optional.h" +#include +using std::optional; #include "point.h" #include "text.h" diff --git a/src/bindings/marker-index-wrapper.cc b/src/bindings/marker-index-wrapper.cc index f53853e3..138e096d 100644 --- a/src/bindings/marker-index-wrapper.cc +++ b/src/bindings/marker-index-wrapper.cc @@ -3,7 +3,8 @@ #include "marker-index.h" #include "nan.h" #include "noop.h" -#include "optional.h" +#include +using std::optional; #include "point-wrapper.h" #include "range.h" diff --git a/src/bindings/marker-index-wrapper.h b/src/bindings/marker-index-wrapper.h index 2f64cc54..f6eaba13 100644 --- a/src/bindings/marker-index-wrapper.h +++ b/src/bindings/marker-index-wrapper.h @@ -1,6 +1,7 @@ #include "nan.h" #include "marker-index.h" -#include "optional.h" +#include +using std::optional; #include "range.h" class MarkerIndexWrapper : public Nan::ObjectWrap { diff --git a/src/bindings/number-conversion.h b/src/bindings/number-conversion.h index cc063be9..3a3f0fc1 100644 --- a/src/bindings/number-conversion.h +++ b/src/bindings/number-conversion.h @@ -2,7 +2,8 @@ #define SUPERSTRING_NUMBER_CONVERSION_H #include "nan.h" -#include "optional.h" +#include +using std::optional; namespace number_conversion { template diff --git a/src/bindings/point-wrapper.h b/src/bindings/point-wrapper.h index fc06b263..52e500fc 100644 --- a/src/bindings/point-wrapper.h +++ b/src/bindings/point-wrapper.h @@ -2,7 +2,8 @@ #define SUPERSTRING_POINT_WRAPPER_H #include "nan.h" -#include "optional.h" +#include +using std::optional; #include "point.h" class PointWrapper : public Nan::ObjectWrap { diff --git a/src/bindings/range-wrapper.h b/src/bindings/range-wrapper.h index de08dd59..9df83e6f 100644 --- a/src/bindings/range-wrapper.h +++ b/src/bindings/range-wrapper.h @@ -2,7 +2,8 @@ #define SUPERSTRING_RANGE_WRAPPER_H #include "nan.h" -#include "optional.h" +#include +using std::optional; #include "point.h" #include "range.h" diff --git a/src/bindings/string-conversion.h b/src/bindings/string-conversion.h index f8178041..61ebfc64 100644 --- a/src/bindings/string-conversion.h +++ b/src/bindings/string-conversion.h @@ -3,7 +3,8 @@ #include #include "nan.h" -#include "optional.h" +#include +using std::optional; #include "text.h" namespace string_conversion { diff --git a/src/core/encoding-conversion.h b/src/core/encoding-conversion.h index 3b91146a..f24303d5 100644 --- a/src/core/encoding-conversion.h +++ b/src/core/encoding-conversion.h @@ -1,7 +1,8 @@ #ifndef SUPERSTRING_ENCODING_CONVERSION_H_ #define SUPERSTRING_ENCODING_CONVERSION_H_ -#include "optional.h" +#include +using std::optional; #include "text.h" #include diff --git a/src/core/optional.h b/src/core/optional.h deleted file mode 100644 index 33852e05..00000000 --- a/src/core/optional.h +++ /dev/null @@ -1,7 +0,0 @@ -#ifndef SUPERSTRING_OPTIONAL_H -#define SUPERSTRING_OPTIONAL_H - -#include -using std::optional; - -#endif // SUPERSTRING_OPTIONAL_H diff --git a/src/core/patch.cc b/src/core/patch.cc index ace85400..422826c2 100644 --- a/src/core/patch.cc +++ b/src/core/patch.cc @@ -1,5 +1,6 @@ #include "patch.h" -#include "optional.h" +#include +using std::optional; #include "text.h" #include "text-slice.h" #include diff --git a/src/core/patch.h b/src/core/patch.h index c59021e5..4ddf8aad 100644 --- a/src/core/patch.h +++ b/src/core/patch.h @@ -1,7 +1,8 @@ #ifndef PATCH_H_ #define PATCH_H_ -#include "optional.h" +#include +using std::optional; #include "point.h" #include "serializer.h" #include "text.h" diff --git a/src/core/regex.h b/src/core/regex.h index 10970306..cd8de961 100644 --- a/src/core/regex.h +++ b/src/core/regex.h @@ -1,7 +1,8 @@ #ifndef REGEX_H_ #define REGEX_H_ -#include "optional.h" +#include +using std::optional; #include struct pcre2_real_code_16; diff --git a/src/core/text.h b/src/core/text.h index 0326a902..4f0f9bb5 100644 --- a/src/core/text.h +++ b/src/core/text.h @@ -7,7 +7,8 @@ #include #include "serializer.h" #include "point.h" -#include "optional.h" +#include +using std::optional; class TextSlice; From 9229018c5f4249be54beb284c6d55eb0805f2e06 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 06:12:03 -0600 Subject: [PATCH 12/40] modernize-deprecated-headers This replaces the deprecated c headers with their versions --- src/bindings/text-buffer-wrapper.cc | 2 +- src/core/encoding-conversion.cc | 2 +- src/core/encoding-conversion.h | 2 +- src/core/libmba-diff.cc | 6 +++--- src/core/libmba-diff.h | 2 +- src/core/marker-index.cc | 2 +- src/core/patch.cc | 4 ++-- src/core/regex.cc | 2 +- src/core/text-diff.cc | 2 +- src/core/text-slice.cc | 2 +- 10 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/bindings/text-buffer-wrapper.cc b/src/bindings/text-buffer-wrapper.cc index f84cf879..bbd37e3c 100644 --- a/src/bindings/text-buffer-wrapper.cc +++ b/src/bindings/text-buffer-wrapper.cc @@ -1,7 +1,7 @@ #include "text-buffer-wrapper.h" #include #include -#include +#include #include "number-conversion.h" #include "point-wrapper.h" #include "range-wrapper.h" diff --git a/src/core/encoding-conversion.cc b/src/core/encoding-conversion.cc index c1e64062..ffb62d68 100644 --- a/src/core/encoding-conversion.cc +++ b/src/core/encoding-conversion.cc @@ -1,7 +1,7 @@ #include "encoding-conversion.h" #include "utf8-conversions.h" #include -#include +#include using std::function; using std::u16string; diff --git a/src/core/encoding-conversion.h b/src/core/encoding-conversion.h index f24303d5..fe667c3d 100644 --- a/src/core/encoding-conversion.h +++ b/src/core/encoding-conversion.h @@ -4,7 +4,7 @@ #include using std::optional; #include "text.h" -#include +#include class EncodingConversion { void *data; diff --git a/src/core/libmba-diff.cc b/src/core/libmba-diff.cc index 418d51a9..7cf09a4d 100644 --- a/src/core/libmba-diff.cc +++ b/src/core/libmba-diff.cc @@ -52,9 +52,9 @@ */ #include "libmba-diff.h" -#include -#include -#include +#include +#include +#include #include using std::vector; diff --git a/src/core/libmba-diff.h b/src/core/libmba-diff.h index 04237974..72e42c0b 100644 --- a/src/core/libmba-diff.h +++ b/src/core/libmba-diff.h @@ -1,7 +1,7 @@ #ifndef MBA_DIFF_H_ #define MBA_DIFF_H_ -#include +#include #include typedef enum { diff --git a/src/core/marker-index.cc b/src/core/marker-index.cc index d622c7ec..b45ccf54 100644 --- a/src/core/marker-index.cc +++ b/src/core/marker-index.cc @@ -2,7 +2,7 @@ #include #include #include -#include +#include #include "range.h" using std::default_random_engine; diff --git a/src/core/patch.cc b/src/core/patch.cc index 422826c2..236cca59 100644 --- a/src/core/patch.cc +++ b/src/core/patch.cc @@ -3,10 +3,10 @@ using std::optional; #include "text.h" #include "text-slice.h" -#include +#include #include #include -#include +#include #include #include diff --git a/src/core/regex.cc b/src/core/regex.cc index 71629642..8c0b9f51 100644 --- a/src/core/regex.cc +++ b/src/core/regex.cc @@ -1,5 +1,5 @@ #include "regex.h" -#include +#include #include "pcre2.h" using std::u16string; diff --git a/src/core/text-diff.cc b/src/core/text-diff.cc index 969b7ea7..e99f2863 100644 --- a/src/core/text-diff.cc +++ b/src/core/text-diff.cc @@ -2,7 +2,7 @@ #include "libmba-diff.h" #include "text-slice.h" #include -#include +#include #include #include diff --git a/src/core/text-slice.cc b/src/core/text-slice.cc index 22f298d7..eb9f8615 100644 --- a/src/core/text-slice.cc +++ b/src/core/text-slice.cc @@ -1,6 +1,6 @@ #include "text-slice.h" #include "text.h" -#include +#include TextSlice::TextSlice() : text{nullptr} {} From d4479ec2636998b1823e471f87a290b2df5a8db0 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 06:16:28 -0600 Subject: [PATCH 13/40] use angle brackets for external headers --- src/bindings/bindings.cc | 2 +- src/bindings/marker-index-wrapper.cc | 2 +- src/bindings/marker-index-wrapper.h | 2 +- src/bindings/noop.h | 2 +- src/bindings/number-conversion.h | 2 +- src/bindings/point-wrapper.cc | 2 +- src/bindings/point-wrapper.h | 2 +- src/bindings/range-wrapper.cc | 2 +- src/bindings/range-wrapper.h | 2 +- src/bindings/string-conversion.h | 2 +- src/bindings/text-buffer-snapshot-wrapper.h | 2 +- src/bindings/text-buffer-wrapper.h | 2 +- src/bindings/text-reader.h | 2 +- src/bindings/text-writer.h | 2 +- src/core/regex.cc | 2 +- 15 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/bindings/bindings.cc b/src/bindings/bindings.cc index fc9b31e1..f58bb520 100644 --- a/src/bindings/bindings.cc +++ b/src/bindings/bindings.cc @@ -1,5 +1,5 @@ #include "marker-index-wrapper.h" -#include "nan.h" +#include #include "patch-wrapper.h" #include "range-wrapper.h" #include "point-wrapper.h" diff --git a/src/bindings/marker-index-wrapper.cc b/src/bindings/marker-index-wrapper.cc index 138e096d..00727f5d 100644 --- a/src/bindings/marker-index-wrapper.cc +++ b/src/bindings/marker-index-wrapper.cc @@ -1,7 +1,7 @@ #include "marker-index-wrapper.h" #include #include "marker-index.h" -#include "nan.h" +#include #include "noop.h" #include using std::optional; diff --git a/src/bindings/marker-index-wrapper.h b/src/bindings/marker-index-wrapper.h index f6eaba13..0b232e01 100644 --- a/src/bindings/marker-index-wrapper.h +++ b/src/bindings/marker-index-wrapper.h @@ -1,4 +1,4 @@ -#include "nan.h" +#include #include "marker-index.h" #include using std::optional; diff --git a/src/bindings/noop.h b/src/bindings/noop.h index 6b446a5a..f02917d9 100644 --- a/src/bindings/noop.h +++ b/src/bindings/noop.h @@ -1,5 +1,5 @@ #pragma once -#include "nan.h" +#include static void noop(const Nan::FunctionCallbackInfo&) {} diff --git a/src/bindings/number-conversion.h b/src/bindings/number-conversion.h index 3a3f0fc1..c0c98b30 100644 --- a/src/bindings/number-conversion.h +++ b/src/bindings/number-conversion.h @@ -1,7 +1,7 @@ #ifndef SUPERSTRING_NUMBER_CONVERSION_H #define SUPERSTRING_NUMBER_CONVERSION_H -#include "nan.h" +#include #include using std::optional; diff --git a/src/bindings/point-wrapper.cc b/src/bindings/point-wrapper.cc index 7bd159f7..9d5a0c25 100644 --- a/src/bindings/point-wrapper.cc +++ b/src/bindings/point-wrapper.cc @@ -1,6 +1,6 @@ #include "point-wrapper.h" #include -#include "nan.h" +#include using namespace v8; diff --git a/src/bindings/point-wrapper.h b/src/bindings/point-wrapper.h index 52e500fc..e380a0b5 100644 --- a/src/bindings/point-wrapper.h +++ b/src/bindings/point-wrapper.h @@ -1,7 +1,7 @@ #ifndef SUPERSTRING_POINT_WRAPPER_H #define SUPERSTRING_POINT_WRAPPER_H -#include "nan.h" +#include #include using std::optional; #include "point.h" diff --git a/src/bindings/range-wrapper.cc b/src/bindings/range-wrapper.cc index b0aaf5ab..72a7393b 100644 --- a/src/bindings/range-wrapper.cc +++ b/src/bindings/range-wrapper.cc @@ -1,6 +1,6 @@ #include "range-wrapper.h" #include "point-wrapper.h" -#include "nan.h" +#include using namespace v8; diff --git a/src/bindings/range-wrapper.h b/src/bindings/range-wrapper.h index 9df83e6f..7755f6fc 100644 --- a/src/bindings/range-wrapper.h +++ b/src/bindings/range-wrapper.h @@ -1,7 +1,7 @@ #ifndef SUPERSTRING_RANGE_WRAPPER_H #define SUPERSTRING_RANGE_WRAPPER_H -#include "nan.h" +#include #include using std::optional; #include "point.h" diff --git a/src/bindings/string-conversion.h b/src/bindings/string-conversion.h index 61ebfc64..abeeeee7 100644 --- a/src/bindings/string-conversion.h +++ b/src/bindings/string-conversion.h @@ -2,7 +2,7 @@ #define SUPERSTRING_STRING_CONVERSION_H #include -#include "nan.h" +#include #include using std::optional; #include "text.h" diff --git a/src/bindings/text-buffer-snapshot-wrapper.h b/src/bindings/text-buffer-snapshot-wrapper.h index bb23b4ce..ee2c7298 100644 --- a/src/bindings/text-buffer-snapshot-wrapper.h +++ b/src/bindings/text-buffer-snapshot-wrapper.h @@ -1,7 +1,7 @@ #ifndef SUPERSTRING_TEXT_BUFFER_SNAPSHOT_WRAPPER_H #define SUPERSTRING_TEXT_BUFFER_SNAPSHOT_WRAPPER_H -#include "nan.h" +#include #include // This header can be included by other native node modules, allowing them diff --git a/src/bindings/text-buffer-wrapper.h b/src/bindings/text-buffer-wrapper.h index ee2261ba..78895456 100644 --- a/src/bindings/text-buffer-wrapper.h +++ b/src/bindings/text-buffer-wrapper.h @@ -1,7 +1,7 @@ #ifndef SUPERSTRING_TEXT_BUFFER_WRAPPER_H #define SUPERSTRING_TEXT_BUFFER_WRAPPER_H -#include "nan.h" +#include #include "text-buffer.h" #include diff --git a/src/bindings/text-reader.h b/src/bindings/text-reader.h index 200b6ce6..7fb6d4b0 100644 --- a/src/bindings/text-reader.h +++ b/src/bindings/text-reader.h @@ -1,7 +1,7 @@ #ifndef SUPERSTRING_TEXT_READER_H #define SUPERSTRING_TEXT_READER_H -#include "nan.h" +#include #include "text.h" #include "text-buffer.h" #include "encoding-conversion.h" diff --git a/src/bindings/text-writer.h b/src/bindings/text-writer.h index 97fde24b..9d699e38 100644 --- a/src/bindings/text-writer.h +++ b/src/bindings/text-writer.h @@ -1,7 +1,7 @@ #ifndef SUPERSTRING_TEXT_WRITER_H #define SUPERSTRING_TEXT_WRITER_H -#include "nan.h" +#include #include "text.h" #include "encoding-conversion.h" diff --git a/src/core/regex.cc b/src/core/regex.cc index 8c0b9f51..c9405d9f 100644 --- a/src/core/regex.cc +++ b/src/core/regex.cc @@ -1,6 +1,6 @@ #include "regex.h" #include -#include "pcre2.h" +#include using std::u16string; using MatchResult = Regex::MatchResult; From 0b9f4c5dcd3ca723e6e94bfad6dcb695522ce97d Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 06:33:30 -0600 Subject: [PATCH 14/40] Make constructors explicit constructors must be marked explicit to avoid unintentional implicit conversions --- src/bindings/patch-wrapper.cc | 2 +- src/bindings/text-buffer-wrapper.cc | 4 ++-- src/core/patch.cc | 2 +- src/core/text-buffer.cc | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/bindings/patch-wrapper.cc b/src/bindings/patch-wrapper.cc index 1cbf7fe2..e8794fcf 100644 --- a/src/bindings/patch-wrapper.cc +++ b/src/bindings/patch-wrapper.cc @@ -65,7 +65,7 @@ class ChangeWrapper : public Nan::ObjectWrap { } private: - ChangeWrapper(Patch::Change change) : change(change) {} + explicit ChangeWrapper(Patch::Change change) : change(change) {} static void construct(const Nan::FunctionCallbackInfo &info) {} diff --git a/src/bindings/text-buffer-wrapper.cc b/src/bindings/text-buffer-wrapper.cc index bbd37e3c..43001e56 100644 --- a/src/bindings/text-buffer-wrapper.cc +++ b/src/bindings/text-buffer-wrapper.cc @@ -78,7 +78,7 @@ class RegexWrapper : public Nan::ObjectWrap { static Nan::Persistent constructor; static void construct(const Nan::FunctionCallbackInfo &info) {} - RegexWrapper(Regex &®ex) : regex{move(regex)} {} + explicit RegexWrapper(Regex &®ex) : regex{move(regex)} {} static const Regex *regex_from_js(const Local &value) { Local js_pattern; @@ -137,7 +137,7 @@ class SubsequenceMatchWrapper : public Nan::ObjectWrap { public: static Nan::Persistent constructor; - SubsequenceMatchWrapper(SubsequenceMatch &&match) : + explicit SubsequenceMatchWrapper(SubsequenceMatch &&match) : match(std::move(match)) {} static void init() { diff --git a/src/core/patch.cc b/src/core/patch.cc index 236cca59..ec06b162 100644 --- a/src/core/patch.cc +++ b/src/core/patch.cc @@ -60,7 +60,7 @@ struct Patch::Node { compute_subtree_text_sizes(); } - Node(Deserializer &input) : + explicit Node(Deserializer &input) : left{nullptr}, right{nullptr}, old_extent{input}, diff --git a/src/core/text-buffer.cc b/src/core/text-buffer.cc index c36d4f1f..22ac44b1 100644 --- a/src/core/text-buffer.cc +++ b/src/core/text-buffer.cc @@ -32,7 +32,7 @@ struct TextBuffer::Layer { uint32_t size_; uint32_t snapshot_count; - Layer(Text &&text) : + explicit Layer(Text &&text) : previous_layer{nullptr}, text{move(text)}, uses_patch{false}, @@ -40,7 +40,7 @@ struct TextBuffer::Layer { size_{this->text->size()}, snapshot_count{0} {} - Layer(Layer *previous_layer) : + explicit Layer(Layer *previous_layer) : previous_layer{previous_layer}, patch{Patch()}, uses_patch{true}, From c7cee01993fb5f3c3656128f2a3b3695e66e2292 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 06:38:19 -0600 Subject: [PATCH 15/40] Make single-argument constructors explicit single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor,hicpp-explicit-conversions] --- src/bindings/patch-wrapper.cc | 2 +- src/bindings/text-buffer-wrapper.cc | 4 ++-- src/core/patch.cc | 2 +- src/core/text-buffer.cc | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/bindings/patch-wrapper.cc b/src/bindings/patch-wrapper.cc index fc610aaf..071afa38 100644 --- a/src/bindings/patch-wrapper.cc +++ b/src/bindings/patch-wrapper.cc @@ -65,7 +65,7 @@ class ChangeWrapper : public Nan::ObjectWrap { } private: - ChangeWrapper(Patch::Change change) : change(change) {} + explicit ChangeWrapper(Patch::Change change) : change(change) {} static void construct(const Nan::FunctionCallbackInfo &info) {} diff --git a/src/bindings/text-buffer-wrapper.cc b/src/bindings/text-buffer-wrapper.cc index 52e7bded..3ae7c8d3 100644 --- a/src/bindings/text-buffer-wrapper.cc +++ b/src/bindings/text-buffer-wrapper.cc @@ -78,7 +78,7 @@ class RegexWrapper : public Nan::ObjectWrap { static Nan::Persistent constructor; static void construct(const Nan::FunctionCallbackInfo &info) {} - RegexWrapper(Regex &®ex) : regex{move(regex)} {} + explicit RegexWrapper(Regex &®ex) : regex{move(regex)} {} static const Regex *regex_from_js(const Local &value) { Local js_pattern; @@ -137,7 +137,7 @@ class SubsequenceMatchWrapper : public Nan::ObjectWrap { public: static Nan::Persistent constructor; - SubsequenceMatchWrapper(SubsequenceMatch &&match) : + explicit SubsequenceMatchWrapper(SubsequenceMatch &&match) : match(std::move(match)) {} static void init() { diff --git a/src/core/patch.cc b/src/core/patch.cc index 8702f9b3..f02bb8b1 100644 --- a/src/core/patch.cc +++ b/src/core/patch.cc @@ -59,7 +59,7 @@ struct Patch::Node { compute_subtree_text_sizes(); } - Node(Deserializer &input) : + explicit Node(Deserializer &input) : left{nullptr}, right{nullptr}, old_extent{input}, diff --git a/src/core/text-buffer.cc b/src/core/text-buffer.cc index c4dd3e8c..9677710b 100644 --- a/src/core/text-buffer.cc +++ b/src/core/text-buffer.cc @@ -32,7 +32,7 @@ struct TextBuffer::Layer { uint32_t size_; uint32_t snapshot_count; - Layer(Text &&text) : + explicit Layer(Text &&text) : previous_layer{nullptr}, text{move(text)}, uses_patch{false}, @@ -40,7 +40,7 @@ struct TextBuffer::Layer { size_{this->text->size()}, snapshot_count{0} {} - Layer(Layer *previous_layer) : + explicit Layer(Layer *previous_layer) : previous_layer{previous_layer}, patch{Patch()}, uses_patch{true}, From 26364d6d74ef53dcf5e7b2041820c1e8998068f1 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 06:43:52 -0600 Subject: [PATCH 16/40] Make multo-argument constructors explicit constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor,hicpp-explicit-conversions] --- src/bindings/text-buffer-wrapper.cc | 14 +++++++------- src/core/serializer.h | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/bindings/text-buffer-wrapper.cc b/src/bindings/text-buffer-wrapper.cc index 3ae7c8d3..eb9ec1e0 100644 --- a/src/bindings/text-buffer-wrapper.cc +++ b/src/bindings/text-buffer-wrapper.cc @@ -401,7 +401,7 @@ class TextBufferSearcher : public Nan::AsyncWorker { Nan::Persistent argument; public: - TextBufferSearcher(Nan::Callback *completion_callback, + explicit TextBufferSearcher(Nan::Callback *completion_callback, const TextBuffer::Snapshot *snapshot, const Regex *regex, const Range &search_range, @@ -742,7 +742,7 @@ class Loader { public: bool cancelled; - Loader(Nan::Callback *progress_callback, Nan::AsyncResource *async_resource, + explicit Loader(Nan::Callback *progress_callback, Nan::AsyncResource *async_resource, TextBuffer *buffer, TextBuffer::Snapshot *snapshot, string &&file_name, string &&encoding_name, bool force, bool compute_patch) : progress_callback{progress_callback}, @@ -755,7 +755,7 @@ class Loader { compute_patch{compute_patch}, cancelled{false} {} - Loader(Nan::Callback *progress_callback, Nan::AsyncResource *async_resource, + explicit Loader(Nan::Callback *progress_callback, Nan::AsyncResource *async_resource, TextBuffer *buffer, TextBuffer::Snapshot *snapshot, Text &&text, bool force, bool compute_patch) : progress_callback{progress_callback}, @@ -848,13 +848,13 @@ class LoadWorker : public Nan::AsyncProgressWorkerBase { Loader loader; public: - LoadWorker(Nan::Callback *completion_callback, Nan::Callback *progress_callback, + explicit LoadWorker(Nan::Callback *completion_callback, Nan::Callback *progress_callback, TextBuffer *buffer, TextBuffer::Snapshot *snapshot, string &&file_name, string &&encoding_name, bool force, bool compute_patch) : AsyncProgressWorkerBase(completion_callback, "TextBuffer.load"), loader(progress_callback, async_resource, buffer, snapshot, move(file_name), move(encoding_name), force, compute_patch) {} - LoadWorker(Nan::Callback *completion_callback, Nan::Callback *progress_callback, + explicit LoadWorker(Nan::Callback *completion_callback, Nan::Callback *progress_callback, TextBuffer *buffer, TextBuffer::Snapshot *snapshot, Text &&text, bool force, bool compute_patch) : AsyncProgressWorkerBase(completion_callback, "TextBuffer.load"), @@ -990,7 +990,7 @@ class BaseTextComparisonWorker : public Nan::AsyncWorker { bool result; public: - BaseTextComparisonWorker(Nan::Callback *completion_callback, TextBuffer::Snapshot *snapshot, + explicit BaseTextComparisonWorker(Nan::Callback *completion_callback, TextBuffer::Snapshot *snapshot, string &&file_name, string &&encoding_name) : AsyncWorker(completion_callback, "TextBuffer.baseTextMatchesFile"), snapshot{snapshot}, @@ -1050,7 +1050,7 @@ class SaveWorker : public Nan::AsyncWorker { optional error; public: - SaveWorker(Nan::Callback *completion_callback, TextBuffer::Snapshot *snapshot, + explicit SaveWorker(Nan::Callback *completion_callback, TextBuffer::Snapshot *snapshot, string &&file_name, string &&encoding_name) : AsyncWorker(completion_callback, "TextBuffer.save"), snapshot{snapshot}, diff --git a/src/core/serializer.h b/src/core/serializer.h index 27b12a4b..2ae8760c 100644 --- a/src/core/serializer.h +++ b/src/core/serializer.h @@ -8,7 +8,7 @@ class Serializer { std::vector &vector; public: - inline Serializer(std::vector &output) : + explicit inline Serializer(std::vector &output) : vector(output) {}; template @@ -25,7 +25,7 @@ class Deserializer { const uint8_t *end_ptr; public: - inline Deserializer(const std::vector &input) : + explicit inline Deserializer(const std::vector &input) : read_ptr(input.data()), end_ptr(input.data() + input.size()) {}; From 851b65c47af7901280efca0fb4e715a72d5c8081 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 06:57:29 -0600 Subject: [PATCH 17/40] Make most of the constructors explicit --- src/bindings/marker-index-wrapper.h | 2 +- src/bindings/patch-wrapper.h | 2 +- src/bindings/point-wrapper.h | 2 +- src/bindings/range-wrapper.h | 2 +- src/bindings/text-buffer-wrapper.cc | 14 +++++++------- src/bindings/text-writer.h | 2 +- src/core/marker-index.h | 4 ++-- src/core/patch.cc | 2 +- src/core/patch.h | 4 ++-- src/core/point.h | 2 +- src/core/regex.h | 4 ++-- src/core/serializer.h | 4 ++-- src/core/text-buffer.h | 4 ++-- 13 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/bindings/marker-index-wrapper.h b/src/bindings/marker-index-wrapper.h index 2f64cc54..90be23bc 100644 --- a/src/bindings/marker-index-wrapper.h +++ b/src/bindings/marker-index-wrapper.h @@ -36,6 +36,6 @@ class MarkerIndexWrapper : public Nan::ObjectWrap { static void find_ending_at(const Nan::FunctionCallbackInfo &info); static void find_boundaries_after(const Nan::FunctionCallbackInfo &info); static void dump(const Nan::FunctionCallbackInfo &info); - MarkerIndexWrapper(unsigned seed); MarkerIndex marker_index; + explicit MarkerIndexWrapper(unsigned seed); }; diff --git a/src/bindings/patch-wrapper.h b/src/bindings/patch-wrapper.h index 97fc6133..d873bdf0 100644 --- a/src/bindings/patch-wrapper.h +++ b/src/bindings/patch-wrapper.h @@ -7,7 +7,7 @@ class PatchWrapper : public Nan::ObjectWrap { static v8::Local from_patch(Patch &&); private: - PatchWrapper(Patch &&patch); + explicit PatchWrapper(Patch &&patch); static void construct(const Nan::FunctionCallbackInfo &info); static void splice(const Nan::FunctionCallbackInfo &info); static void splice_old(const Nan::FunctionCallbackInfo &info); diff --git a/src/bindings/point-wrapper.h b/src/bindings/point-wrapper.h index fc06b263..c9771d96 100644 --- a/src/bindings/point-wrapper.h +++ b/src/bindings/point-wrapper.h @@ -12,7 +12,7 @@ class PointWrapper : public Nan::ObjectWrap { static optional point_from_js(v8::Local); private: - PointWrapper(Point point); + explicit PointWrapper(Point point); static void construct(const Nan::FunctionCallbackInfo &info); diff --git a/src/bindings/range-wrapper.h b/src/bindings/range-wrapper.h index de08dd59..b2bf0228 100644 --- a/src/bindings/range-wrapper.h +++ b/src/bindings/range-wrapper.h @@ -13,7 +13,7 @@ class RangeWrapper : public Nan::ObjectWrap { static optional range_from_js(v8::Local); private: - RangeWrapper(Range); + explicit RangeWrapper(Range); static void construct(const Nan::FunctionCallbackInfo &); static void get_start(v8::Local, const Nan::PropertyCallbackInfo &); diff --git a/src/bindings/text-buffer-wrapper.cc b/src/bindings/text-buffer-wrapper.cc index 3ae7c8d3..eb9ec1e0 100644 --- a/src/bindings/text-buffer-wrapper.cc +++ b/src/bindings/text-buffer-wrapper.cc @@ -401,7 +401,7 @@ class TextBufferSearcher : public Nan::AsyncWorker { Nan::Persistent argument; public: - TextBufferSearcher(Nan::Callback *completion_callback, + explicit TextBufferSearcher(Nan::Callback *completion_callback, const TextBuffer::Snapshot *snapshot, const Regex *regex, const Range &search_range, @@ -742,7 +742,7 @@ class Loader { public: bool cancelled; - Loader(Nan::Callback *progress_callback, Nan::AsyncResource *async_resource, + explicit Loader(Nan::Callback *progress_callback, Nan::AsyncResource *async_resource, TextBuffer *buffer, TextBuffer::Snapshot *snapshot, string &&file_name, string &&encoding_name, bool force, bool compute_patch) : progress_callback{progress_callback}, @@ -755,7 +755,7 @@ class Loader { compute_patch{compute_patch}, cancelled{false} {} - Loader(Nan::Callback *progress_callback, Nan::AsyncResource *async_resource, + explicit Loader(Nan::Callback *progress_callback, Nan::AsyncResource *async_resource, TextBuffer *buffer, TextBuffer::Snapshot *snapshot, Text &&text, bool force, bool compute_patch) : progress_callback{progress_callback}, @@ -848,13 +848,13 @@ class LoadWorker : public Nan::AsyncProgressWorkerBase { Loader loader; public: - LoadWorker(Nan::Callback *completion_callback, Nan::Callback *progress_callback, + explicit LoadWorker(Nan::Callback *completion_callback, Nan::Callback *progress_callback, TextBuffer *buffer, TextBuffer::Snapshot *snapshot, string &&file_name, string &&encoding_name, bool force, bool compute_patch) : AsyncProgressWorkerBase(completion_callback, "TextBuffer.load"), loader(progress_callback, async_resource, buffer, snapshot, move(file_name), move(encoding_name), force, compute_patch) {} - LoadWorker(Nan::Callback *completion_callback, Nan::Callback *progress_callback, + explicit LoadWorker(Nan::Callback *completion_callback, Nan::Callback *progress_callback, TextBuffer *buffer, TextBuffer::Snapshot *snapshot, Text &&text, bool force, bool compute_patch) : AsyncProgressWorkerBase(completion_callback, "TextBuffer.load"), @@ -990,7 +990,7 @@ class BaseTextComparisonWorker : public Nan::AsyncWorker { bool result; public: - BaseTextComparisonWorker(Nan::Callback *completion_callback, TextBuffer::Snapshot *snapshot, + explicit BaseTextComparisonWorker(Nan::Callback *completion_callback, TextBuffer::Snapshot *snapshot, string &&file_name, string &&encoding_name) : AsyncWorker(completion_callback, "TextBuffer.baseTextMatchesFile"), snapshot{snapshot}, @@ -1050,7 +1050,7 @@ class SaveWorker : public Nan::AsyncWorker { optional error; public: - SaveWorker(Nan::Callback *completion_callback, TextBuffer::Snapshot *snapshot, + explicit SaveWorker(Nan::Callback *completion_callback, TextBuffer::Snapshot *snapshot, string &&file_name, string &&encoding_name) : AsyncWorker(completion_callback, "TextBuffer.save"), snapshot{snapshot}, diff --git a/src/bindings/text-writer.h b/src/bindings/text-writer.h index 97fde24b..4adc7c77 100644 --- a/src/bindings/text-writer.h +++ b/src/bindings/text-writer.h @@ -8,7 +8,7 @@ class TextWriter : public Nan::ObjectWrap { public: static void init(v8::Local exports); - TextWriter(EncodingConversion &&conversion); + explicit TextWriter(EncodingConversion &&conversion); std::u16string get_text(); private: diff --git a/src/core/marker-index.h b/src/core/marker-index.h index 4b231156..74dd8893 100644 --- a/src/core/marker-index.h +++ b/src/core/marker-index.h @@ -30,7 +30,7 @@ class MarkerIndex { std::vector boundaries; }; - MarkerIndex(unsigned seed = 0u); + explicit MarkerIndex(unsigned seed = 0u); ~MarkerIndex(); int generate_random_number(); void insert(MarkerId id, Point start, Point end); @@ -74,7 +74,7 @@ class MarkerIndex { class Iterator { public: - Iterator(MarkerIndex *marker_index); + explicit Iterator(MarkerIndex *marker_index); void reset(); Node* insert_marker_start(const MarkerId &id, const Point &start_position, const Point &end_position); Node* insert_marker_end(const MarkerId &id, const Point &start_position, const Point &end_position); diff --git a/src/core/patch.cc b/src/core/patch.cc index f02bb8b1..c64821ee 100644 --- a/src/core/patch.cc +++ b/src/core/patch.cc @@ -36,7 +36,7 @@ struct Patch::Node { uint32_t old_subtree_text_size; uint32_t new_subtree_text_size; - Node( + explicit Node( Node *left, Node *right, Point old_extent, diff --git a/src/core/patch.h b/src/core/patch.h index c59021e5..26f466bf 100644 --- a/src/core/patch.h +++ b/src/core/patch.h @@ -35,10 +35,10 @@ class Patch { }; // Construction and destruction - Patch(bool merges_adjacent_changes = true); Patch(Patch &&); - Patch(Deserializer &input); Patch &operator=(Patch &&); + explicit Patch(bool merges_adjacent_changes = true); + explicit Patch(Deserializer &input); ~Patch(); void serialize(Serializer &serializer); diff --git a/src/core/point.h b/src/core/point.h index b896bf08..b47ee49f 100644 --- a/src/core/point.h +++ b/src/core/point.h @@ -14,7 +14,7 @@ struct Point { Point(); Point(unsigned row, unsigned column); - Point(Deserializer &input); + explicit Point(Deserializer &input); int compare(const Point &other) const; bool is_zero() const; diff --git a/src/core/regex.h b/src/core/regex.h index 10970306..ba4229b8 100644 --- a/src/core/regex.h +++ b/src/core/regex.h @@ -10,7 +10,7 @@ struct BuildRegexResult; class Regex { pcre2_real_code_16 *code; - Regex(pcre2_real_code_16 *); + explicit Regex(pcre2_real_code_16 *); public: Regex(); @@ -24,7 +24,7 @@ class Regex { friend class Regex; public: - MatchData(const Regex &); + explicit MatchData(const Regex & /*regex*/); ~MatchData(); }; diff --git a/src/core/serializer.h b/src/core/serializer.h index 27b12a4b..2ae8760c 100644 --- a/src/core/serializer.h +++ b/src/core/serializer.h @@ -8,7 +8,7 @@ class Serializer { std::vector &vector; public: - inline Serializer(std::vector &output) : + explicit inline Serializer(std::vector &output) : vector(output) {}; template @@ -25,7 +25,7 @@ class Deserializer { const uint8_t *end_ptr; public: - inline Deserializer(const std::vector &input) : + explicit inline Deserializer(const std::vector &input) : read_ptr(input.data()), end_ptr(input.data() + input.size()) {}; diff --git a/src/core/text-buffer.h b/src/core/text-buffer.h index 5b2800cb..30dc1f95 100644 --- a/src/core/text-buffer.h +++ b/src/core/text-buffer.h @@ -21,8 +21,8 @@ class TextBuffer { static uint32_t MAX_CHUNK_SIZE_TO_COPY; TextBuffer(); - TextBuffer(std::u16string &&); - TextBuffer(const std::u16string &text); + explicit TextBuffer(std::u16string && /*text*/); + explicit TextBuffer(const std::u16string &text); ~TextBuffer(); uint32_t size() const; From f98a830b6ba77f355b90e79d6f0d78a76ea582a0 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 07:00:59 -0600 Subject: [PATCH 18/40] Add TODO for making TextSlice explicit --- src/core/text-slice.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/text-slice.h b/src/core/text-slice.h index b4263010..d6e9ed07 100644 --- a/src/core/text-slice.h +++ b/src/core/text-slice.h @@ -16,8 +16,9 @@ class TextSlice { size_t start_offset() const; size_t end_offset() const; - TextSlice(); - TextSlice(const Text &text); + explicit TextSlice(); + TextSlice(const Text &text); // TODO make it explicit + std::pair split(Point) const; std::pair split(uint32_t) const; TextSlice prefix(Point) const; From 5e64ae43ca71d0806dc9b67b0a62bc1dd20abf79 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 07:02:33 -0600 Subject: [PATCH 19/40] Add TODO for making Text explicit --- src/core/text.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/text.h b/src/core/text.h index 0326a902..2430828a 100644 --- a/src/core/text.h +++ b/src/core/text.h @@ -28,11 +28,11 @@ class Text { using const_iterator = std::u16string::const_iterator; - Text(); - Text(const std::u16string &); - Text(std::u16string &&); - Text(TextSlice slice); - Text(Deserializer &deserializer); + explicit Text(); + explicit Text(const std::u16string & /*string*/); + Text(std::u16string && /*content*/); // TODO make it explicit + explicit Text(TextSlice slice); + explicit Text(Deserializer &deserializer); template Text(Iter begin, Iter end) : Text(std::u16string{begin, end}) {} From d73adf77dc964b79e997e76e9928cd6b89b529e5 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 07:10:57 -0600 Subject: [PATCH 20/40] Use Clang 10 on Travis --- .travis.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1831c829..ae46e398 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,7 +12,7 @@ node_js: - "12.14.1" before_install: - - export CXX="g++-4.9" CC="gcc-4.9" + - export CXX="clang-10.0" CC="clang++-10.0" script: - npm run standard @@ -31,5 +31,5 @@ addons: sources: - ubuntu-toolchain-r-test packages: - - gcc-4.9 - - g++-4.9 + - clang-10.0 + - clang++-10.0 From d50be88857fadd0e71efe2ec874907496d97a982 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 07:21:45 -0600 Subject: [PATCH 21/40] explicit comparison for ferror to prevent conversion Fixes implicit conversion 'int' -> bool --- src/core/encoding-conversion.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/encoding-conversion.cc b/src/core/encoding-conversion.cc index ffb62d68..f60b6759 100644 --- a/src/core/encoding-conversion.cc +++ b/src/core/encoding-conversion.cc @@ -149,7 +149,7 @@ bool EncodingConversion::decode(u16string &string, FILE *stream, for (;;) { size_t bytes_to_read = input_vector.size() - bytes_left_over; size_t bytes_read = fread(input_buffer + bytes_left_over, 1, bytes_to_read, stream); - if (bytes_read < bytes_to_read && ferror(stream)) return false; + if (bytes_read < bytes_to_read && (ferror(stream) != 0)) return false; size_t bytes_to_append = bytes_left_over + bytes_read; if (bytes_to_append == 0) break; @@ -259,7 +259,7 @@ bool EncodingConversion::encode(const u16string &string, size_t start_offset, } } size_t bytes_written = fwrite(output_buffer, 1, bytes_encoded, stream); - if (bytes_written < bytes_encoded && ferror(stream)) return false; + if (bytes_written < bytes_encoded && (ferror(stream) != 0)) return false; } return true; From 349bdd8dec34bea4ae30cfd5328766b6445332ad Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 07:23:18 -0600 Subject: [PATCH 22/40] explicit comparison for int/uint numbers To prevent implicit conversion 'int/uint' -> bool --- src/core/libmba-diff.cc | 8 ++++---- src/core/marker-index.cc | 14 +++++++------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/core/libmba-diff.cc b/src/core/libmba-diff.cc index 7cf09a4d..dca69cda 100644 --- a/src/core/libmba-diff.cc +++ b/src/core/libmba-diff.cc @@ -125,7 +125,7 @@ static int _find_middle_snake( } _setv(ctx, k, 0, x); - if (odd && k >= (delta - (d - 1)) && k <= (delta + (d - 1))) { + if ((odd != 0) && k >= (delta - (d - 1)) && k <= (delta + (d - 1))) { if (x >= RV(k)) { ms->u = x; ms->v = y; @@ -153,7 +153,7 @@ static int _find_middle_snake( } _setv(ctx, kr, 1, x); - if (!odd && kr >= -d && kr <= d) { + if ((odd == 0) && kr >= -d && kr <= d) { if (x <= FV(kr)) { ms->x = x; ms->y = y; @@ -172,7 +172,7 @@ static void _edit(struct _ctx *ctx, diff_op op, int off, int len) { // Add an edit to the SES (or coalesce if the op is the same) auto *e = &ctx->ses->back(); if (e->op != op) { - if (e->op) { + if (e->op != 0) { ctx->ses->push_back(diff_edit{}); e = &ctx->ses->back(); } @@ -267,7 +267,7 @@ int diff(const char16_t *a, uint32_t n, const char16_t *b, uint32_t m, int dmax, vector *ses) { struct _ctx ctx; ctx.ses = ses; - ctx.dmax = dmax ? dmax : INT_MAX; + ctx.dmax = dmax != 0 ? dmax : INT_MAX; ses->push_back(diff_edit{static_cast(0), 0, 0}); uint32_t common_prefix_length = 0; diff --git a/src/core/marker-index.cc b/src/core/marker-index.cc index b45ccf54..1ea07cb9 100644 --- a/src/core/marker-index.cc +++ b/src/core/marker-index.cc @@ -547,14 +547,14 @@ MarkerIndex::SpliceResult MarkerIndex::splice(Point start, Point old_extent, Poi for (MarkerId id : ending_inside_splice) { end_node->end_marker_ids.insert(id); - if (!starting_inside_splice.count(id)) { + if (starting_inside_splice.count(id) == 0u) { start_node->right_marker_ids.insert(id); } end_nodes_by_id[id] = end_node; } for (MarkerId id : end_node->end_marker_ids) { - if (exclusive_marker_ids.count(id) && !end_node->start_marker_ids.count(id)) { + if ((exclusive_marker_ids.count(id) != 0u) && (end_node->start_marker_ids.count(id) == 0u)) { ending_inside_splice.insert(id); } } @@ -566,7 +566,7 @@ MarkerIndex::SpliceResult MarkerIndex::splice(Point start, Point old_extent, Poi for (auto iter = start_node->start_marker_ids.begin(); iter != start_node->start_marker_ids.end();) { MarkerId id = *iter; - if (exclusive_marker_ids.count(id) && !start_node->end_marker_ids.count(id)) { + if ((exclusive_marker_ids.count(id) != 0u) && (start_node->end_marker_ids.count(id) == 0u)) { iter = start_node->start_marker_ids.erase(iter); start_node->right_marker_ids.erase(id); end_node->start_marker_ids.insert(id); @@ -807,7 +807,7 @@ void MarkerIndex::rotate_node_left(Node *rotation_pivot) { rotation_pivot->right_marker_ids.insert(rotation_root->right_marker_ids.begin(), rotation_root->right_marker_ids.end()); for (auto it = rotation_pivot->left_marker_ids.begin(); it != rotation_pivot->left_marker_ids.end();) { - if (rotation_root->left_marker_ids.count(*it)) { + if (rotation_root->left_marker_ids.count(*it) != 0u) { rotation_root->left_marker_ids.erase(*it); ++it; } else { @@ -842,13 +842,13 @@ void MarkerIndex::rotate_node_right(Node *rotation_pivot) { rotation_root->left_extent = rotation_root->left_extent.traversal(rotation_pivot->left_extent); for (auto it = rotation_root->left_marker_ids.begin(); it != rotation_root->left_marker_ids.end(); ++it) { - if (!rotation_pivot->start_marker_ids.count(*it)) { + if (rotation_pivot->start_marker_ids.count(*it) == 0u) { rotation_pivot->left_marker_ids.insert(*it); } } for (auto it = rotation_pivot->right_marker_ids.begin(); it != rotation_pivot->right_marker_ids.end();) { - if (rotation_root->right_marker_ids.count(*it)) { + if (rotation_root->right_marker_ids.count(*it) != 0u) { rotation_root->right_marker_ids.erase(*it); ++it; } else { @@ -887,7 +887,7 @@ void MarkerIndex::populate_splice_invalidation_sets(SpliceResult *invalidated, c invalidated->touch.insert(id); invalidated->inside.insert(id); invalidated->overlap.insert(id); - if (ending_inside_splice.count(id)) invalidated->surround.insert(id); + if (ending_inside_splice.count(id) != 0u) invalidated->surround.insert(id); } for (MarkerId id : ending_inside_splice) { From d661ac93e929d89094c0f7ffca9595193115df05 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 3 Dec 2020 07:24:04 -0600 Subject: [PATCH 23/40] explicit comparison for objects that may be null Prevents implicit conversion of the object to bool --- src/bindings/string-conversion.cc | 4 +- src/bindings/text-buffer-snapshot-wrapper.cc | 2 +- src/core/encoding-conversion.cc | 2 +- src/core/marker-index.cc | 98 ++++++++-------- src/core/patch.cc | 116 +++++++++---------- src/core/regex.cc | 4 +- src/core/text-buffer.cc | 18 +-- 7 files changed, 122 insertions(+), 122 deletions(-) diff --git a/src/bindings/string-conversion.cc b/src/bindings/string-conversion.cc index 669feac4..bd1bb8fe 100644 --- a/src/bindings/string-conversion.cc +++ b/src/bindings/string-conversion.cc @@ -36,7 +36,7 @@ Local string_conversion::string_to_js(const u16string &text, const char ).ToLocal(&result)) { return result; } else { - if (!failure_message) failure_message = "Couldn't convert text to a String"; + if (failure_message == nullptr) failure_message = "Couldn't convert text to a String"; Nan::ThrowError(failure_message); return Nan::New("").ToLocalChecked(); } @@ -47,7 +47,7 @@ Local string_conversion::char_to_js(const uint16_t c, const char *failur if (Nan::New(&c, 1).ToLocal(&result)) { return result; } else { - if (!failure_message) failure_message = "Couldn't convert character to a String"; + if (failure_message == nullptr) failure_message = "Couldn't convert character to a String"; Nan::ThrowError(failure_message); return Nan::New("").ToLocalChecked(); } diff --git a/src/bindings/text-buffer-snapshot-wrapper.cc b/src/bindings/text-buffer-snapshot-wrapper.cc index d613b6a6..52e21d6d 100644 --- a/src/bindings/text-buffer-snapshot-wrapper.cc +++ b/src/bindings/text-buffer-snapshot-wrapper.cc @@ -26,7 +26,7 @@ TextBufferSnapshotWrapper::TextBufferSnapshotWrapper(Local js_buffer, vo } TextBufferSnapshotWrapper::~TextBufferSnapshotWrapper() { - if (snapshot) { + if (snapshot != nullptr) { delete reinterpret_cast(snapshot); } } diff --git a/src/core/encoding-conversion.cc b/src/core/encoding-conversion.cc index f60b6759..932d6c96 100644 --- a/src/core/encoding-conversion.cc +++ b/src/core/encoding-conversion.cc @@ -61,7 +61,7 @@ EncodingConversion::EncodingConversion(int mode, void *data) : data{data}, mode{mode} {} EncodingConversion::~EncodingConversion() { - if (data) iconv_close(data); + if (data != nullptr) iconv_close(data); } int EncodingConversion::convert( diff --git a/src/core/marker-index.cc b/src/core/marker-index.cc index 1ea07cb9..fc24dab4 100644 --- a/src/core/marker-index.cc +++ b/src/core/marker-index.cc @@ -25,7 +25,7 @@ MarkerIndex::Iterator::Iterator(MarkerIndex *marker_index) : void MarkerIndex::Iterator::reset() { current_node = marker_index->root; - if (current_node) { + if (current_node != nullptr) { current_node_position = current_node->left_extent; } left_ancestor_position = Point(0, 0); @@ -37,7 +37,7 @@ void MarkerIndex::Iterator::reset() { MarkerIndex::Node *MarkerIndex::Iterator::insert_marker_start(const MarkerId &id, const Point &start_position, const Point &end_position) { reset(); - if (!current_node) { + if (current_node == nullptr) { return marker_index->root = new Node(nullptr, start_position); } @@ -48,7 +48,7 @@ MarkerIndex::Node *MarkerIndex::Iterator::insert_marker_start(const MarkerId &id return current_node; case -1: mark_right(id, start_position, end_position); - if (current_node->left) { + if (current_node->left != nullptr) { descend_left(); break; } else { @@ -58,7 +58,7 @@ MarkerIndex::Node *MarkerIndex::Iterator::insert_marker_start(const MarkerId &id return current_node; } case 1: - if (current_node->right) { + if (current_node->right != nullptr) { descend_right(); break; } else { @@ -74,7 +74,7 @@ MarkerIndex::Node *MarkerIndex::Iterator::insert_marker_start(const MarkerId &id MarkerIndex::Node *MarkerIndex::Iterator::insert_marker_end(const MarkerId &id, const Point &start_position, const Point &end_position) { reset(); - if (!current_node) { + if (current_node == nullptr) { return marker_index->root = new Node(nullptr, end_position); } @@ -84,7 +84,7 @@ MarkerIndex::Node *MarkerIndex::Iterator::insert_marker_end(const MarkerId &id, mark_left(id, start_position, end_position); return current_node; case -1: - if (current_node->left) { + if (current_node->left != nullptr) { descend_left(); break; } else { @@ -95,7 +95,7 @@ MarkerIndex::Node *MarkerIndex::Iterator::insert_marker_end(const MarkerId &id, } case 1: mark_left(id, start_position, end_position); - if (current_node->right) { + if (current_node->right != nullptr) { descend_right(); break; } else { @@ -116,14 +116,14 @@ MarkerIndex::Node *MarkerIndex::Iterator::insert_splice_boundary(const Point &po if (comparison == 0 && !is_insertion_end) { return current_node; } else if (comparison < 0) { - if (current_node->left) { + if (current_node->left != nullptr) { descend_left(); } else { insert_left_child(position); return current_node->left; } } else { // comparison > 0 - if (current_node->right) { + if (current_node->right != nullptr) { descend_right(); } else { insert_right_child(position); @@ -136,19 +136,19 @@ MarkerIndex::Node *MarkerIndex::Iterator::insert_splice_boundary(const Point &po void MarkerIndex::Iterator::find_intersecting(const Point &start, const Point &end, MarkerIdSet *result) { reset(); - if (!current_node) return; + if (current_node == nullptr) return; while (true) { cache_node_position(); if (start < current_node_position) { - if (current_node->left) { + if (current_node->left != nullptr) { check_intersection(start, end, result); descend_left(); } else { break; } } else { - if (current_node->right) { + if (current_node->right != nullptr) { check_intersection(start, end, result); descend_right(); } else { @@ -161,18 +161,18 @@ void MarkerIndex::Iterator::find_intersecting(const Point &start, const Point &e check_intersection(start, end, result); move_to_successor(); cache_node_position(); - } while (current_node && current_node_position <= end); + } while ((current_node != nullptr) && current_node_position <= end); } void MarkerIndex::Iterator::find_contained_in(const Point &start, const Point &end, MarkerIdSet *result) { reset(); - if (!current_node) return; + if (current_node == nullptr) return; seek_to_first_node_greater_than_or_equal_to(start); MarkerIdSet started; - while (current_node && current_node_position <= end) { + while ((current_node != nullptr) && current_node_position <= end) { started.insert(current_node->start_marker_ids.begin(), current_node->start_marker_ids.end()); for (MarkerId id : current_node->end_marker_ids) { if (started.count(id) > 0) result->insert(id); @@ -185,11 +185,11 @@ void MarkerIndex::Iterator::find_contained_in(const Point &start, const Point &e void MarkerIndex::Iterator::find_starting_in(const Point &start, const Point &end, MarkerIdSet *result) { reset(); - if (!current_node) return; + if (current_node == nullptr) return; seek_to_first_node_greater_than_or_equal_to(start); - while (current_node && current_node_position <= end) { + while ((current_node != nullptr) && current_node_position <= end) { result->insert(current_node->start_marker_ids.begin(), current_node->start_marker_ids.end()); cache_node_position(); move_to_successor(); @@ -199,11 +199,11 @@ void MarkerIndex::Iterator::find_starting_in(const Point &start, const Point &en void MarkerIndex::Iterator::find_ending_in(const Point &start, const Point &end, MarkerIdSet *result) { reset(); - if (!current_node) return; + if (current_node == nullptr) return; seek_to_first_node_greater_than_or_equal_to(start); - while (current_node && current_node_position <= end) { + while ((current_node != nullptr) && current_node_position <= end) { result->insert(current_node->end_marker_ids.begin(), current_node->end_marker_ids.end()); cache_node_position(); move_to_successor(); @@ -212,7 +212,7 @@ void MarkerIndex::Iterator::find_ending_in(const Point &start, const Point &end, void MarkerIndex::Iterator::find_boundaries_after(Point start, size_t max_count, MarkerIndex::BoundaryQueryResult *result) { reset(); - if (!current_node) return; + if (current_node == nullptr) return; while (true) { cache_node_position(); @@ -226,7 +226,7 @@ void MarkerIndex::Iterator::find_boundaries_after(Point start, size_t max_count, ); } - if (current_node->left) { + if (current_node->left != nullptr) { descend_left(); } else { break; @@ -240,7 +240,7 @@ void MarkerIndex::Iterator::find_boundaries_after(Point start, size_t max_count, ); } - if (current_node->right) { + if (current_node->right != nullptr) { descend_right(); } else { break; @@ -257,7 +257,7 @@ void MarkerIndex::Iterator::find_boundaries_after(Point start, size_t max_count, ); if (current_node_position < start) move_to_successor(); - while (current_node && max_count > 0) { + while ((current_node != nullptr) && max_count > 0) { cache_node_position(); result->boundaries.push_back({ current_node_position, @@ -274,14 +274,14 @@ unordered_map MarkerIndex::Iterator::dump() { unordered_map snapshot; - if (!current_node) return snapshot; + if (current_node == nullptr) return snapshot; - while (current_node && current_node->left) { + while ((current_node != nullptr) && (current_node->left != nullptr)) { cache_node_position(); descend_left(); } - while (current_node) { + while (current_node != nullptr) { for (MarkerId id : current_node->start_marker_ids) { Range range; range.start = current_node_position; @@ -300,7 +300,7 @@ unordered_map MarkerIndex::Iterator::dump() { } void MarkerIndex::Iterator::ascend() { - if (current_node->parent) { + if (current_node->parent != nullptr) { if (current_node->parent->left == current_node) { current_node_position = right_ancestor_position; } else { @@ -338,15 +338,15 @@ void MarkerIndex::Iterator::descend_right() { } void MarkerIndex::Iterator::move_to_successor() { - if (!current_node) return; + if (current_node == nullptr) return; - if (current_node->right) { + if (current_node->right != nullptr) { descend_right(); - while (current_node->left) { + while (current_node->left != nullptr) { descend_left(); } } else { - while (current_node->parent && current_node->parent->right == current_node) { + while ((current_node->parent != nullptr) && current_node->parent->right == current_node) { ascend(); } ascend(); @@ -359,13 +359,13 @@ void MarkerIndex::Iterator::seek_to_first_node_greater_than_or_equal_to(const Po if (position == current_node_position) { break; } else if (position < current_node_position) { - if (current_node->left) { + if (current_node->left != nullptr) { descend_left(); } else { break; } } else { // position > current_node_position - if (current_node->right) { + if (current_node->right != nullptr) { descend_right(); } else { break; @@ -424,7 +424,7 @@ MarkerIndex::MarkerIndex(unsigned seed) iterator{this} {} MarkerIndex::~MarkerIndex() { - if (root) delete_subtree(root); + if (root != nullptr) delete_subtree(root); } int MarkerIndex::generate_random_number() { @@ -468,13 +468,13 @@ void MarkerIndex::remove(MarkerId id) { Node *end_node = end_nodes_by_id.find(id)->second; Node *node = start_node; - while (node) { + while (node != nullptr) { node->right_marker_ids.erase(id); node = node->parent; } node = end_node; - while (node) { + while (node != nullptr) { node->left_marker_ids.erase(id); node = node->parent; } @@ -503,7 +503,7 @@ MarkerIndex::SpliceResult MarkerIndex::splice(Point start, Point old_extent, Poi SpliceResult invalidated; - if (!root || (old_extent.is_zero() && new_extent.is_zero())) return invalidated; + if ((root == nullptr) || (old_extent.is_zero() && new_extent.is_zero())) return invalidated; bool is_insertion = old_extent.is_zero(); Node *start_node = iterator.insert_splice_boundary(start, false); @@ -580,7 +580,7 @@ MarkerIndex::SpliceResult MarkerIndex::splice(Point start, Point old_extent, Poi populate_splice_invalidation_sets(&invalidated, start_node, end_node, starting_inside_splice, ending_inside_splice); - if (start_node->right) { + if (start_node->right != nullptr) { delete_subtree(start_node->right); start_node->right = nullptr; } @@ -716,7 +716,7 @@ Point MarkerIndex::get_node_position(const Node *node) const { if (cache_entry == node_position_cache.end()) { Point position = node->left_extent; const Node *current_node = node; - while (current_node->parent) { + while (current_node->parent != nullptr) { if (current_node->parent->right == current_node) { position = current_node->parent->left_extent.traverse(position); } @@ -736,7 +736,7 @@ void MarkerIndex::delete_node(Node *node) { bubble_node_down(node); - if (node->parent) { + if (node->parent != nullptr) { if (node->parent->left == node) { node->parent->left = nullptr; } else { @@ -750,13 +750,13 @@ void MarkerIndex::delete_node(Node *node) { } void MarkerIndex::delete_subtree(Node *node) { - if (node->left) delete_subtree(node->left); - if (node->right) delete_subtree(node->right); + if (node->left != nullptr) delete_subtree(node->left); + if (node->right != nullptr) delete_subtree(node->right); delete node; } void MarkerIndex::bubble_node_up(Node *node) { - while (node->parent && node->priority < node->parent->priority) { + while ((node->parent != nullptr) && node->priority < node->parent->priority) { if (node == node->parent->left) { rotate_node_right(node); } else { @@ -767,8 +767,8 @@ void MarkerIndex::bubble_node_up(Node *node) { void MarkerIndex::bubble_node_down(Node *node) { while (true) { - int left_child_priority = (node->left) ? node->left->priority : INT_MAX; - int right_child_priority = (node->right) ? node->right->priority : INT_MAX; + int left_child_priority = (node->left) != nullptr ? node->left->priority : INT_MAX; + int right_child_priority = (node->right) != nullptr ? node->right->priority : INT_MAX; if (left_child_priority < right_child_priority && left_child_priority < node->priority) { rotate_node_right(node->left); @@ -783,7 +783,7 @@ void MarkerIndex::bubble_node_down(Node *node) { void MarkerIndex::rotate_node_left(Node *rotation_pivot) { Node *rotation_root = rotation_pivot->parent; - if (rotation_root->parent) { + if (rotation_root->parent != nullptr) { if (rotation_root->parent->left == rotation_root) { rotation_root->parent->left = rotation_pivot; } else { @@ -795,7 +795,7 @@ void MarkerIndex::rotate_node_left(Node *rotation_pivot) { rotation_pivot->parent = rotation_root->parent; rotation_root->right = rotation_pivot->left; - if (rotation_root->right) { + if (rotation_root->right != nullptr) { rotation_root->right->parent = rotation_root; } @@ -820,7 +820,7 @@ void MarkerIndex::rotate_node_left(Node *rotation_pivot) { void MarkerIndex::rotate_node_right(Node *rotation_pivot) { Node *rotation_root = rotation_pivot->parent; - if (rotation_root->parent) { + if (rotation_root->parent != nullptr) { if (rotation_root->parent->left == rotation_root) { rotation_root->parent->left = rotation_pivot; } else { @@ -832,7 +832,7 @@ void MarkerIndex::rotate_node_right(Node *rotation_pivot) { rotation_pivot->parent = rotation_root->parent; rotation_root->left = rotation_pivot->right; - if (rotation_root->left) { + if (rotation_root->left != nullptr) { rotation_root->left->parent = rotation_root; } diff --git a/src/core/patch.cc b/src/core/patch.cc index 02ee3f25..f0923562 100644 --- a/src/core/patch.cc +++ b/src/core/patch.cc @@ -105,11 +105,11 @@ struct Patch::Node { } uint32_t left_subtree_old_text_size() const { - return left ? left->old_subtree_text_size : 0; + return left != nullptr ? left->old_subtree_text_size : 0; } uint32_t right_subtree_old_text_size() const { - return right ? right->old_subtree_text_size : 0; + return right != nullptr ? right->old_subtree_text_size : 0; } void set_new_text(optional &&text) { @@ -125,18 +125,18 @@ struct Patch::Node { } uint32_t left_subtree_new_text_size() const { - return left ? left->new_subtree_text_size : 0; + return left != nullptr ? left->new_subtree_text_size : 0; } uint32_t right_subtree_new_text_size() const { - return right ? right->new_subtree_text_size : 0; + return right != nullptr ? right->new_subtree_text_size : 0; } void get_subtree_end(Point *old_end, Point *new_end) { Node *node = this; *old_end = Point(); *new_end = Point(); - while (node) { + while (node != nullptr) { *old_end = old_end->traverse(node->old_distance_from_left_ancestor) .traverse(node->old_extent); *new_end = new_end->traverse(node->new_distance_from_left_ancestor) @@ -233,7 +233,7 @@ struct Patch::Node { << "\"]" << endl; result << "node_" << this << " -> "; - if (left) { + if (left != nullptr) { result << "node_" << left << endl; left->write_dot_graph(result, left_ancestor_old_end, left_ancestor_new_end); } else { @@ -242,7 +242,7 @@ struct Patch::Node { } result << "node_" << this << " -> "; - if (right) { + if (right != nullptr) { result << "node_" << right << endl; right->write_dot_graph(result, node_old_end, node_new_end); } else { @@ -264,14 +264,14 @@ struct Patch::Node { result << "\"newStart\": {\"row\": " << node_new_start.row << ", \"column\": " << node_new_start.column << "}, "; result << "\"newEnd\": {\"row\": " << node_new_end.row << ", \"column\": " << node_new_end.column << "}, "; result << "\"left\": "; - if (left) { + if (left != nullptr) { left->write_json(result, left_ancestor_old_end, left_ancestor_new_end); } else { result << "null"; } result << ", "; result << "\"right\": "; - if (right) { + if (right != nullptr) { right->write_json(result, node_old_end, node_new_end); } else { result << "null"; @@ -388,28 +388,28 @@ Patch &Patch::operator=(Patch &&other) { } Patch::~Patch() { - if (root) delete_node(&root); + if (root != nullptr) delete_node(&root); } void Patch::serialize(Serializer &output) { output.append(SERIALIZATION_VERSION); output.append(change_count); - if (!root) return; + if (root == nullptr) return; root->serialize(output); Node *node = root; node_stack.clear(); int previous_node_child_index = -1; - while (node) { - if (node->left && previous_node_child_index < 0) { + while (node != nullptr) { + if ((node->left != nullptr) && previous_node_child_index < 0) { output.append(Left); node->left->serialize(output); node_stack.push_back(node); node = node->left; previous_node_child_index = -1; - } else if (node->right && previous_node_child_index < 1) { + } else if ((node->right != nullptr) && previous_node_child_index < 1) { output.append(Right); node->right->serialize(output); node_stack.push_back(node); @@ -429,7 +429,7 @@ void Patch::serialize(Serializer &output) { Patch Patch::copy() { Node *new_root = nullptr; - if (root) { + if (root != nullptr) { new_root = root->copy(); node_stack.clear(); node_stack.push_back(new_root); @@ -437,11 +437,11 @@ Patch Patch::copy() { while (!node_stack.empty()) { Node *node = node_stack.back(); node_stack.pop_back(); - if (node->left) { + if (node->left != nullptr) { node->left = node->left->copy(); node_stack.push_back(node->left); } - if (node->right) { + if (node->right != nullptr) { node->right = node->right->copy(); node_stack.push_back(node->right); } @@ -453,7 +453,7 @@ Patch Patch::copy() { Patch Patch::invert() { Node *inverted_root = nullptr; - if (root) { + if (root != nullptr) { inverted_root = root->invert(); node_stack.clear(); node_stack.push_back(inverted_root); @@ -461,11 +461,11 @@ Patch Patch::invert() { while (!node_stack.empty()) { Node *node = node_stack.back(); node_stack.pop_back(); - if (node->left) { + if (node->left != nullptr) { node->left = node->left->invert(); node_stack.push_back(node->left); } - if (node->right) { + if (node->right != nullptr) { node->right = node->right->invert(); node_stack.push_back(node->right); } @@ -483,7 +483,7 @@ bool Patch::splice(Point new_splice_start, uint32_t deleted_text_size) { if (new_deletion_extent.is_zero() && new_insertion_extent.is_zero()) return true; - if (!root) { + if (root == nullptr) { root = build_node(nullptr, nullptr, new_splice_start, new_splice_start, new_deletion_extent, new_insertion_extent, move(deleted_text), move(inserted_text), @@ -506,13 +506,13 @@ bool Patch::splice(Point new_splice_start, } Node *upper_bound = splay_node_ending_after(new_deletion_end, new_splice_start); - if (upper_bound && lower_bound && lower_bound != upper_bound) { + if ((upper_bound != nullptr) && (lower_bound != nullptr) && lower_bound != upper_bound) { if (lower_bound != upper_bound->left) { rotate_node_right(lower_bound, upper_bound->left, upper_bound); } } - if (lower_bound && upper_bound) { + if ((lower_bound != nullptr) && (upper_bound != nullptr)) { Point lower_bound_old_start = lower_bound->old_distance_from_left_ancestor; Point lower_bound_new_start = lower_bound->new_distance_from_left_ancestor; Point upper_bound_old_start = upper_bound->old_distance_from_left_ancestor; @@ -659,7 +659,7 @@ bool Patch::splice(Point new_splice_start, upper_bound_new_start.traversal(new_deletion_end); } } - } else if (lower_bound) { + } else if (lower_bound != nullptr) { Point lower_bound_old_start = lower_bound->old_distance_from_left_ancestor; Point lower_bound_new_start = lower_bound->new_distance_from_left_ancestor; Point lower_bound_new_end = @@ -702,7 +702,7 @@ bool Patch::splice(Point new_splice_start, } delete_node(&lower_bound->right); - } else if (upper_bound) { + } else if (upper_bound != nullptr) { Point upper_bound_new_start = upper_bound->new_distance_from_left_ancestor; Point upper_bound_old_start = upper_bound->old_distance_from_left_ancestor; Point upper_bound_new_end = @@ -712,7 +712,7 @@ bool Patch::splice(Point new_splice_start, (merges_adjacent_changes && new_deletion_end == upper_bound_new_start); Point old_deletion_end; - if (upper_bound->left) { + if (upper_bound->left != nullptr) { Point rightmost_child_old_end, rightmost_child_new_end; upper_bound->left->get_subtree_end(&rightmost_child_old_end, &rightmost_child_new_end); old_deletion_end = @@ -771,15 +771,15 @@ bool Patch::splice(Point new_splice_start, ); } - if (lower_bound) lower_bound->compute_subtree_text_sizes(); - if (upper_bound) upper_bound->compute_subtree_text_sizes(); + if (lower_bound != nullptr) lower_bound->compute_subtree_text_sizes(); + if (upper_bound != nullptr) upper_bound->compute_subtree_text_sizes(); return true; } void Patch::splice_old(Point old_splice_start, Point old_deletion_extent, Point old_insertion_extent) { - if (!root) return; + if (root == nullptr) return; Point old_deletion_end = old_splice_start.traverse(old_deletion_extent); Point old_insertion_end = old_splice_start.traverse(old_insertion_extent); @@ -787,7 +787,7 @@ void Patch::splice_old(Point old_splice_start, Point old_deletion_extent, Node *lower_bound = splay_node_ending_before(old_splice_start); Node *upper_bound = splay_node_starting_after(old_deletion_end, old_splice_start); - if (!lower_bound && !upper_bound) { + if ((lower_bound == nullptr) && (upper_bound == nullptr)) { delete_node(&root); return; } @@ -802,7 +802,7 @@ void Patch::splice_old(Point old_splice_start, Point old_deletion_extent, return; } - if (upper_bound && lower_bound) { + if ((upper_bound != nullptr) && (lower_bound != nullptr)) { if (lower_bound != upper_bound->left) { rotate_node_right(lower_bound, upper_bound->left, upper_bound); } @@ -810,7 +810,7 @@ void Patch::splice_old(Point old_splice_start, Point old_deletion_extent, Point new_deletion_end, new_insertion_end; - if (lower_bound) { + if (lower_bound != nullptr) { Point lower_bound_old_start = lower_bound->old_distance_from_left_ancestor; Point lower_bound_new_start = lower_bound->new_distance_from_left_ancestor; Point lower_bound_old_end = @@ -828,7 +828,7 @@ void Patch::splice_old(Point old_splice_start, Point old_deletion_extent, new_insertion_end = old_insertion_end; } - if (upper_bound) { + if (upper_bound != nullptr) { Point distance_between_splice_and_upper_bound = upper_bound->old_distance_from_left_ancestor.traversal( old_deletion_end); @@ -837,7 +837,7 @@ void Patch::splice_old(Point old_splice_start, Point old_deletion_extent, upper_bound->new_distance_from_left_ancestor = new_insertion_end.traverse(distance_between_splice_and_upper_bound); - if (lower_bound) { + if (lower_bound != nullptr) { if (lower_bound->old_distance_from_left_ancestor.traverse( lower_bound->old_extent) == upper_bound->old_distance_from_left_ancestor) { @@ -875,8 +875,8 @@ void Patch::splice_old(Point old_splice_start, Point old_deletion_extent, } } - if (lower_bound) lower_bound->compute_subtree_text_sizes(); - if (upper_bound) upper_bound->compute_subtree_text_sizes(); + if (lower_bound != nullptr) lower_bound->compute_subtree_text_sizes(); + if (upper_bound != nullptr) upper_bound->compute_subtree_text_sizes(); } bool Patch::combine(const Patch &other, bool left_to_right) { @@ -905,19 +905,19 @@ bool Patch::combine(const Patch &other, bool left_to_right) { } void Patch::clear() { - if (root) delete_node(&root); + if (root != nullptr) delete_node(&root); } void Patch::rebalance() { - if (!root) + if (root == nullptr) return; // Transform tree to vine Node *pseudo_root = root, *pseudo_root_parent = nullptr; - while (pseudo_root) { + while (pseudo_root != nullptr) { Node *left = pseudo_root->left; Node *right = pseudo_root->right; - if (left) { + if (left != nullptr) { rotate_node_right(left, pseudo_root, pseudo_root_parent); pseudo_root = left; } else { @@ -952,7 +952,7 @@ optional Patch::get_bounds() const { if (!root) return optional{}; Node *node = root; - while (node->left) { + while (node->left != nullptr) { node = node->left; } @@ -961,7 +961,7 @@ optional Patch::get_bounds() const { node = root; Point old_end, new_end; - while (node) { + while (node != nullptr) { old_end = old_end.traverse(node->old_distance_from_left_ancestor.traverse(node->old_extent)); new_end = new_end.traverse(node->new_distance_from_left_ancestor.traverse(node->new_extent)); node = node->right; @@ -1003,7 +1003,7 @@ Point Patch::new_position_for_new_offset(uint32_t target_offset, Point preceding_new_position, preceding_old_position; uint32_t preceding_old_offset = 0, preceding_new_offset = 0; - while (node) { + while (node != nullptr) { Point node_old_start = left_ancestor_info.old_end.traverse(node->old_distance_from_left_ancestor); Point node_new_start = left_ancestor_info.new_end.traverse(node->new_distance_from_left_ancestor); uint32_t node_old_start_offset = old_offset_for_old_position(node_old_start); @@ -1020,7 +1020,7 @@ Point Patch::new_position_for_new_offset(uint32_t target_offset, preceding_new_position = node_new_start.traverse(node->new_extent); preceding_old_offset = node_old_end_offset; preceding_new_offset = node_new_end_offset; - if (node->right) { + if (node->right != nullptr) { left_ancestor_info.old_end = preceding_old_position; left_ancestor_info.new_end = preceding_new_position; left_ancestor_info.total_old_text_size += node->left_subtree_old_text_size() + node->old_text_size(); @@ -1033,7 +1033,7 @@ Point Patch::new_position_for_new_offset(uint32_t target_offset, return node_new_start .traverse(node->new_text->position_for_offset(target_offset - node_new_start_offset)); } else { - if (node->left) { + if (node->left != nullptr) { node = node->left; } else { break; @@ -1105,7 +1105,7 @@ void Patch::splay_node(Node *node) { node_stack.pop_back(); } - if (grandparent) { + if (grandparent != nullptr) { Node *great_grandparent = nullptr; if (!node_stack.empty()) { great_grandparent = node_stack.back(); @@ -1137,7 +1137,7 @@ void Patch::splay_node(Node *node) { } void Patch::rotate_node_left(Node *pivot, Node *root, Node *root_parent) { - if (root_parent) { + if (root_parent != nullptr) { if (root_parent->left == root) { root_parent->left = pivot; } else { @@ -1162,7 +1162,7 @@ void Patch::rotate_node_left(Node *pivot, Node *root, Node *root_parent) { } void Patch::rotate_node_right(Node *pivot, Node *root, Node *root_parent) { - if (root_parent) { + if (root_parent != nullptr) { if (root_parent->left == root) { root_parent->left = pivot; } else { @@ -1189,15 +1189,15 @@ void Patch::rotate_node_right(Node *pivot, Node *root, Node *root_parent) { void Patch::delete_root() { Node *node = root, *parent = nullptr; while (true) { - if (node->left) { + if (node->left != nullptr) { Node *left = node->left; rotate_node_right(node->left, node, parent); parent = left; - } else if (node->right) { + } else if (node->right != nullptr) { Node *right = node->right; rotate_node_left(node->right, node, parent); parent = right; - } else if (parent) { + } else if (parent != nullptr) { if (parent->left == node) { delete_node(&parent->left); break; @@ -1218,10 +1218,10 @@ void Patch::delete_root() { void Patch::perform_rebalancing_rotations(uint32_t count) { Node *pseudo_root = root, *pseudo_root_parent = nullptr; for (uint32_t i = 0; i < count; i++) { - if (!pseudo_root) + if (pseudo_root == nullptr) return; Node *right_child = pseudo_root->right; - if (!right_child) + if (right_child == nullptr) return; rotate_node_left(right_child, pseudo_root, pseudo_root_parent); pseudo_root = right_child->right; @@ -1321,16 +1321,16 @@ Patch::Node *Patch::build_node(Node *left, Node *right, } void Patch::delete_node(Node **node_to_delete) { - if (*node_to_delete) { + if (*node_to_delete != nullptr) { node_stack.clear(); node_stack.push_back(*node_to_delete); while (!node_stack.empty()) { Node *node = node_stack.back(); node_stack.pop_back(); - if (node->left) + if (node->left != nullptr) node_stack.push_back(node->left); - if (node->right) + if (node->right != nullptr) node_stack.push_back(node->right); delete node; change_count--; @@ -1862,7 +1862,7 @@ ostream &operator<<(ostream &stream, const Patch::Change &change) { << ", new_range: (" << change.new_start << " - " << change.new_end << ")" << ", old_text: "; - if (change.old_text) { + if (change.old_text != nullptr) { stream << *change.old_text; } else { stream << "null"; @@ -1870,7 +1870,7 @@ ostream &operator<<(ostream &stream, const Patch::Change &change) { stream << ", new_text: "; - if (change.new_text) { + if (change.new_text != nullptr) { stream << *change.new_text; } else { stream << "null"; diff --git a/src/core/regex.cc b/src/core/regex.cc index c9405d9f..c2c89149 100644 --- a/src/core/regex.cc +++ b/src/core/regex.cc @@ -69,7 +69,7 @@ Regex::Regex(const char16_t *pattern, uint32_t pattern_length, u16string *error_ nullptr ); - if (!code) { + if (code == nullptr) { uint16_t message_buffer[256]; size_t length = pcre2_get_error_message(error_number, message_buffer, 256); error_message->assign(message_buffer, message_buffer + length); @@ -90,7 +90,7 @@ Regex::Regex(Regex &&other) : code{other.code} { } Regex::~Regex() { - if (code) pcre2_code_free(code); + if (code != nullptr) pcre2_code_free(code); } Regex::MatchData::MatchData(const Regex ®ex) diff --git a/src/core/text-buffer.cc b/src/core/text-buffer.cc index 22ac44b1..bacd6619 100644 --- a/src/core/text-buffer.cc +++ b/src/core/text-buffer.cc @@ -54,7 +54,7 @@ struct TextBuffer::Layer { bool is_above_layer(const Layer *layer) const { Layer *predecessor = previous_layer; - while (predecessor) { + while (predecessor != nullptr) { if (predecessor == layer) return true; predecessor = predecessor->previous_layer; } @@ -680,7 +680,7 @@ TextBuffer::TextBuffer() : TextBuffer::~TextBuffer() { Layer *layer = top_layer; - while (layer) { + while (layer != nullptr) { Layer *previous_layer = layer->previous_layer; delete layer; layer = previous_layer; @@ -693,7 +693,7 @@ TextBuffer::TextBuffer(const std::u16string &text) : void TextBuffer::reset(Text &&new_base_text) { bool has_snapshot = false; auto layer = top_layer; - while (layer) { + while (layer != nullptr) { if (layer->snapshot_count > 0) { has_snapshot = true; break; @@ -708,7 +708,7 @@ void TextBuffer::reset(Text &&new_base_text) { } layer = top_layer->previous_layer; - while (layer) { + while (layer != nullptr) { Layer *previous_layer = layer->previous_layer; delete layer; layer = previous_layer; @@ -783,7 +783,7 @@ void TextBuffer::serialize_changes(Serializer &serializer) { } bool TextBuffer::deserialize_changes(Deserializer &deserializer) { - if (top_layer != base_layer || base_layer->previous_layer) return false; + if (top_layer != base_layer || (base_layer->previous_layer != nullptr)) return false; top_layer = new Layer(base_layer); top_layer->size_ = deserializer.read(); top_layer->extent_ = Point(deserializer); @@ -975,7 +975,7 @@ bool TextBuffer::is_modified(const Snapshot *snapshot) const { string TextBuffer::get_dot_graph() const { Layer *layer = top_layer; vector layers; - while (layer) { + while (layer != nullptr) { layers.push_back(layer); layer = layer->previous_layer; } @@ -999,7 +999,7 @@ string TextBuffer::get_dot_graph() const { size_t TextBuffer::layer_count() const { size_t result = 1; const Layer *layer = top_layer; - while (layer->previous_layer) { + while (layer->previous_layer != nullptr) { result++; layer = layer->previous_layer; } @@ -1094,7 +1094,7 @@ void TextBuffer::consolidate_layers() { vector mutable_layers; bool needed_by_layer_above = false; - while (layer) { + while (layer != nullptr) { if (needed_by_layer_above || layer->snapshot_count > 0) { squash_layers(mutable_layers); mutable_layers.clear(); @@ -1149,7 +1149,7 @@ void TextBuffer::squash_layers(const vector &layers) { Patch patch; Layer *previous_layer = layers.back()->previous_layer; - if (previous_layer) { + if (previous_layer != nullptr) { layer_index = layer_count - 1; patch = move(layers[layer_index]->patch); layer_index--; From f4602c96c244d3582ad1b5c363bcddf58f6b106f Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 8 Dec 2020 08:23:30 -0600 Subject: [PATCH 24/40] install download instead of unzip package --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 9ea8aa1b..148e0f4f 100644 --- a/package.json +++ b/package.json @@ -35,11 +35,11 @@ }, "devDependencies": { "chai": "^2.0.0", + "download": "^8.0.0", "mocha": "^2.3.4", "random-seed": "^0.2.0", "standard": "^4.5.4", - "temp": "^0.8.3", - "unzip": "^0.1.11" + "temp": "^0.8.3" }, "standard": { "global": [ From 8d8f32354284cc3ec5bae47a4d39c2365e48bf6b Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 8 Dec 2020 08:24:04 -0600 Subject: [PATCH 25/40] fix getText by using download package --- .gitignore | 2 ++ benchmark/large-text-buffer.benchmark.js | 45 +++++++++--------------- 2 files changed, 19 insertions(+), 28 deletions(-) diff --git a/.gitignore b/.gitignore index 51f142db..53fdf6b5 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,5 @@ build /browser.js emsdk-portable package-lock.json + +benchmark/*.csv diff --git a/benchmark/large-text-buffer.benchmark.js b/benchmark/large-text-buffer.benchmark.js index 95106bb3..6cac11d0 100644 --- a/benchmark/large-text-buffer.benchmark.js +++ b/benchmark/large-text-buffer.benchmark.js @@ -1,33 +1,22 @@ -const http = require('http') -const fs = require('fs') -const unzip = require('unzip') const { TextBuffer } = require('..') -const unzipper = unzip.Parse() - -const getText = () => { - return new Promise(resolve => { - console.log('fetching text file...') - const req = http.get({ - hostname: 'www.acleddata.com', - port: 80, - // 51 MB text file - path: '/wp-content/uploads/2017/01/ACLED-Version-7-All-Africa-1997-2016_csv_dyadic-file.zip', - agent: false - }, res => { - res - .pipe(unzipper) - .on('entry', entry => { - let data = ''; - entry.on('data', chunk => data += chunk); - entry.on('end', () => { - resolve(data) - }); - }) - }) - - req.end() - }) +const fs = require('fs') +const {promisify} = require('util') +const readFile = promisify(fs.readFile) +const path = require('path') +const download = require('download') + +async function getText() { + const filePath = path.join(__dirname, '1000000 Sales Records.csv') + if (!fs.existsSync(filePath)) { + // 122MB file + await download( + 'http://eforexcel.com/wp/wp-content/uploads/2017/07/1000000%20Sales%20Records.zip', + __dirname, + {extract: true} + ) + } + return await readFile(filePath) } const timer = size => `Time to find "cat" in ${size} file` From 592a10dc4ec8b9ce63361cc1e7b97d61a9c0d398 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 8 Dec 2020 08:24:14 -0600 Subject: [PATCH 26/40] run the benchmark for the full size --- benchmark/large-text-buffer.benchmark.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmark/large-text-buffer.benchmark.js b/benchmark/large-text-buffer.benchmark.js index 6cac11d0..37378617 100644 --- a/benchmark/large-text-buffer.benchmark.js +++ b/benchmark/large-text-buffer.benchmark.js @@ -26,7 +26,7 @@ getText().then(txt => { console.log('running findWordsWithSubsequence tests...') - const sizes = [['10b', 10], ['100b', 100], ['1kb', 1000], ['1MB', 1000000], ['51MB', 100000000]] + const sizes = [['10b', 10], ['100b', 100], ['1kb', 1000], ['1MB', 1000000], ['51MB', 100000000], ['119MB', txt.length]] const test = size => { const _timer = timer(size[0]) From 38a70893e85675832c4cb20f8bcdd18c1bf032e4 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 8 Dec 2020 08:25:32 -0600 Subject: [PATCH 27/40] add large-text benchmark to npm benchmark script --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 148e0f4f..e53f5415 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "test:node": "mocha test/js/*.js", "test:browser": "SUPERSTRING_USE_BROWSER_VERSION=1 mocha test/js/*.js", "test": "npm run test:node && npm run test:browser", - "benchmark": "node benchmark/marker-index.benchmark.js", + "benchmark": "node benchmark/marker-index.benchmark.js && node benchmark/large-text-buffer.benchmark.js", "prepublishOnly": "git submodule update --init --recursive && npm run build:browser", "standard": "standard --recursive src test" }, From f4eb0a5f59aaa0528a06fe244b2dd46bdf3122a5 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 8 Dec 2020 08:45:13 -0600 Subject: [PATCH 28/40] use performance from perf_hooks to measure time --- benchmark/large-text-buffer.benchmark.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/benchmark/large-text-buffer.benchmark.js b/benchmark/large-text-buffer.benchmark.js index 37378617..0e9ad125 100644 --- a/benchmark/large-text-buffer.benchmark.js +++ b/benchmark/large-text-buffer.benchmark.js @@ -5,6 +5,7 @@ const {promisify} = require('util') const readFile = promisify(fs.readFile) const path = require('path') const download = require('download') +const {performance} = require("perf_hooks") async function getText() { const filePath = path.join(__dirname, '1000000 Sales Records.csv') @@ -19,8 +20,6 @@ async function getText() { return await readFile(filePath) } -const timer = size => `Time to find "cat" in ${size} file` - getText().then(txt => { const buffer = new TextBuffer() @@ -28,17 +27,18 @@ getText().then(txt => { const sizes = [['10b', 10], ['100b', 100], ['1kb', 1000], ['1MB', 1000000], ['51MB', 100000000], ['119MB', txt.length]] - const test = size => { - const _timer = timer(size[0]) + const word = "Morocco" + const test = (word, size) => { buffer.setText(txt.slice(0, size[1])) - console.time(_timer) - return buffer.findWordsWithSubsequence('cat', '', 100).then(sugs => { - console.timeEnd(_timer) + const ti = performance.now() + return buffer.findWordsWithSubsequence(word, '', 100).then(sugs => { + const tf = performance.now() + console.log(`Time to find "${word}" in ${size[0]} file: ${(tf-ti).toFixed(5)} ms`) }) } return sizes.reduce((promise, size) => { - return promise.then(() => test(size)) + return promise.then(() => test(word, size)) }, Promise.resolve()) }).then(() => { console.log('finished') From 6cbc91244ba29f12f77e9a874a713bd84c8590be Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 8 Dec 2020 08:45:39 -0600 Subject: [PATCH 29/40] remove 10b test (Very small to find anything meaningful) --- benchmark/large-text-buffer.benchmark.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmark/large-text-buffer.benchmark.js b/benchmark/large-text-buffer.benchmark.js index 0e9ad125..f833aa4b 100644 --- a/benchmark/large-text-buffer.benchmark.js +++ b/benchmark/large-text-buffer.benchmark.js @@ -25,7 +25,7 @@ getText().then(txt => { console.log('running findWordsWithSubsequence tests...') - const sizes = [['10b', 10], ['100b', 100], ['1kb', 1000], ['1MB', 1000000], ['51MB', 100000000], ['119MB', txt.length]] + const sizes = [ ['100b', 100], ['1kb', 1000], ['1MB', 1000000], ['51MB', 100000000], ['119MB', txt.length]] const word = "Morocco" const test = (word, size) => { From 79d838ce1bc98ed4e4e5d215cab54376b6e82af0 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 8 Dec 2020 08:48:26 -0600 Subject: [PATCH 30/40] run the tests for different words --- benchmark/large-text-buffer.benchmark.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/benchmark/large-text-buffer.benchmark.js b/benchmark/large-text-buffer.benchmark.js index f833aa4b..eb0f1ad2 100644 --- a/benchmark/large-text-buffer.benchmark.js +++ b/benchmark/large-text-buffer.benchmark.js @@ -23,11 +23,10 @@ async function getText() { getText().then(txt => { const buffer = new TextBuffer() - console.log('running findWordsWithSubsequence tests...') + console.log('\n running findWordsWithSubsequence tests... \n') const sizes = [ ['100b', 100], ['1kb', 1000], ['1MB', 1000000], ['51MB', 100000000], ['119MB', txt.length]] - const word = "Morocco" const test = (word, size) => { buffer.setText(txt.slice(0, size[1])) const ti = performance.now() @@ -37,9 +36,11 @@ getText().then(txt => { }) } - return sizes.reduce((promise, size) => { - return promise.then(() => test(word, size)) - }, Promise.resolve()) + for (const word of ["Morocco", "Austria", "France", "Liechtenstein", "Republic of the Congo", "Antigua and Barbuda", "Japan"]) { + sizes.reduce((promise, size) => { + return promise.then(() => test(word, size)) + }, Promise.resolve()) + } }).then(() => { - console.log('finished') + console.log('findWordsWithSubsequence tests finished \n') }) From 93348883b97482710c170584a8ddc79e6c23359a Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 8 Dec 2020 08:57:59 -0600 Subject: [PATCH 31/40] pretty print the result --- benchmark/large-text-buffer.benchmark.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmark/large-text-buffer.benchmark.js b/benchmark/large-text-buffer.benchmark.js index eb0f1ad2..81babe25 100644 --- a/benchmark/large-text-buffer.benchmark.js +++ b/benchmark/large-text-buffer.benchmark.js @@ -32,7 +32,7 @@ getText().then(txt => { const ti = performance.now() return buffer.findWordsWithSubsequence(word, '', 100).then(sugs => { const tf = performance.now() - console.log(`Time to find "${word}" in ${size[0]} file: ${(tf-ti).toFixed(5)} ms`) + console.log(`In ${size[0]} file, time to find "${word}" was: ${' '.repeat(50-word.length-size[0].length)} ${(tf-ti).toFixed(5)} ms`) }) } From 3ab437dbab7fa2c1aeee089291d4ce03fba31ab1 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 8 Dec 2020 09:02:58 -0600 Subject: [PATCH 32/40] make test runner async + run same size tests together --- benchmark/large-text-buffer.benchmark.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/benchmark/large-text-buffer.benchmark.js b/benchmark/large-text-buffer.benchmark.js index 81babe25..c69d6139 100644 --- a/benchmark/large-text-buffer.benchmark.js +++ b/benchmark/large-text-buffer.benchmark.js @@ -20,7 +20,7 @@ async function getText() { return await readFile(filePath) } -getText().then(txt => { +getText().then(async (txt) => { const buffer = new TextBuffer() console.log('\n running findWordsWithSubsequence tests... \n') @@ -35,11 +35,11 @@ getText().then(txt => { console.log(`In ${size[0]} file, time to find "${word}" was: ${' '.repeat(50-word.length-size[0].length)} ${(tf-ti).toFixed(5)} ms`) }) } - - for (const word of ["Morocco", "Austria", "France", "Liechtenstein", "Republic of the Congo", "Antigua and Barbuda", "Japan"]) { - sizes.reduce((promise, size) => { - return promise.then(() => test(word, size)) - }, Promise.resolve()) + for (const size of sizes) { + for (const word of ["Morocco", "Austria", "France", "Liechtenstein", "Republic of the Congo", "Antigua and Barbuda", "Japan"]) { + await test(word, size) + } + console.log('\n') } }).then(() => { console.log('findWordsWithSubsequence tests finished \n') From 154867658f2195565a2bff58ba6cfcfac3ad5dbf Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 8 Dec 2020 09:05:00 -0600 Subject: [PATCH 33/40] make test function async --- benchmark/large-text-buffer.benchmark.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/benchmark/large-text-buffer.benchmark.js b/benchmark/large-text-buffer.benchmark.js index c69d6139..5a0f0cee 100644 --- a/benchmark/large-text-buffer.benchmark.js +++ b/benchmark/large-text-buffer.benchmark.js @@ -27,13 +27,12 @@ getText().then(async (txt) => { const sizes = [ ['100b', 100], ['1kb', 1000], ['1MB', 1000000], ['51MB', 100000000], ['119MB', txt.length]] - const test = (word, size) => { + const test = async (word, size) => { buffer.setText(txt.slice(0, size[1])) const ti = performance.now() - return buffer.findWordsWithSubsequence(word, '', 100).then(sugs => { - const tf = performance.now() - console.log(`In ${size[0]} file, time to find "${word}" was: ${' '.repeat(50-word.length-size[0].length)} ${(tf-ti).toFixed(5)} ms`) - }) + await buffer.findWordsWithSubsequence(word, '', 100) + const tf = performance.now() + console.log(`In ${size[0]} file, time to find "${word}" was: ${' '.repeat(50-word.length-size[0].length)} ${(tf-ti).toFixed(5)} ms`) } for (const size of sizes) { for (const word of ["Morocco", "Austria", "France", "Liechtenstein", "Republic of the Congo", "Antigua and Barbuda", "Japan"]) { From 928fa1cea66631b4dd08bccb247816c22b5d1fbf Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 8 Dec 2020 09:55:06 -0600 Subject: [PATCH 34/40] benchmark buffer.setText --- benchmark/large-text-buffer.benchmark.js | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/benchmark/large-text-buffer.benchmark.js b/benchmark/large-text-buffer.benchmark.js index 5a0f0cee..1fca4037 100644 --- a/benchmark/large-text-buffer.benchmark.js +++ b/benchmark/large-text-buffer.benchmark.js @@ -23,23 +23,31 @@ async function getText() { getText().then(async (txt) => { const buffer = new TextBuffer() - console.log('\n running findWordsWithSubsequence tests... \n') + console.log('\n running large-text-buffer tests... \n') const sizes = [ ['100b', 100], ['1kb', 1000], ['1MB', 1000000], ['51MB', 100000000], ['119MB', txt.length]] const test = async (word, size) => { - buffer.setText(txt.slice(0, size[1])) - const ti = performance.now() + const ti2 = performance.now() await buffer.findWordsWithSubsequence(word, '', 100) - const tf = performance.now() - console.log(`In ${size[0]} file, time to find "${word}" was: ${' '.repeat(50-word.length-size[0].length)} ${(tf-ti).toFixed(5)} ms`) + const tf2 = performance.now() + console.log(`For ${size[0]} file, time to find "${word}" was: ${' '.repeat(50-word.length-size[0].length)} ${(tf2-ti2).toFixed(5)} ms`) } for (const size of sizes) { + + const bufferText = txt.slice(0, size[1]) + + // benchmark buffer.setText + const ti1 = performance.now() + buffer.setText(bufferText) + const tf1 = performance.now() + console.log(`For ${size[0]} file, buffer.setText took ${' '.repeat(51-size[0].length)} ${(tf1-ti1).toFixed(5)} ms`) + for (const word of ["Morocco", "Austria", "France", "Liechtenstein", "Republic of the Congo", "Antigua and Barbuda", "Japan"]) { await test(word, size) } console.log('\n') } }).then(() => { - console.log('findWordsWithSubsequence tests finished \n') + console.log(' large-text-buffer finished \n') }) From cdd5de27adff19f27efc0c994c77df951abaaa84 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 8 Dec 2020 16:54:32 -0600 Subject: [PATCH 35/40] fix text-buffer benchmark --- benchmark/text-buffer.benchmark.js | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/benchmark/text-buffer.benchmark.js b/benchmark/text-buffer.benchmark.js index 36152b1f..10e98605 100644 --- a/benchmark/text-buffer.benchmark.js +++ b/benchmark/text-buffer.benchmark.js @@ -10,7 +10,7 @@ function benchmarkSearch(description, pattern, expectedPosition) { let name = `Search for ${description} - TextBuffer` console.time(name) for (let i = 0; i < trialCount; i++) { - assert.deepEqual(buffer.searchSync(pattern), expectedPosition) + assert.deepEqual(buffer.findSync(pattern), expectedPosition) } console.timeEnd(name) diff --git a/package.json b/package.json index e53f5415..c5ad7dcb 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "test:node": "mocha test/js/*.js", "test:browser": "SUPERSTRING_USE_BROWSER_VERSION=1 mocha test/js/*.js", "test": "npm run test:node && npm run test:browser", - "benchmark": "node benchmark/marker-index.benchmark.js && node benchmark/large-text-buffer.benchmark.js", + "benchmark": "node benchmark/text-buffer.benchmark.js && node benchmark/marker-index.benchmark.js && node benchmark/large-text-buffer.benchmark.js", "prepublishOnly": "git submodule update --init --recursive && npm run build:browser", "standard": "standard --recursive src test" }, From f5792c73f62a7540918d4d23ce281a2fd0ee03f2 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 8 Dec 2020 17:03:46 -0600 Subject: [PATCH 36/40] use performance in text-buffer benchmarks + prettry print --- benchmark/text-buffer.benchmark.js | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/benchmark/text-buffer.benchmark.js b/benchmark/text-buffer.benchmark.js index 10e98605..21b63147 100644 --- a/benchmark/text-buffer.benchmark.js +++ b/benchmark/text-buffer.benchmark.js @@ -1,4 +1,8 @@ +console.log(' running text-buffer tests... \n') + const assert = require('assert') +const {performance} = require("perf_hooks") + const {TextBuffer} = require('..') const text = 'abc def ghi jkl\n'.repeat(1024 * 1024) @@ -8,14 +12,15 @@ const trialCount = 10 function benchmarkSearch(description, pattern, expectedPosition) { let name = `Search for ${description} - TextBuffer` - console.time(name) + const ti1 = performance.now() for (let i = 0; i < trialCount; i++) { assert.deepEqual(buffer.findSync(pattern), expectedPosition) } - console.timeEnd(name) + const tf1 = performance.now() + console.log(`${name} ${' '.repeat(80-name.length)} ${(tf1-ti1).toFixed(3)} ms`) name = `Search for ${description} - lines array` - console.time(name) + const ti2 = performance.now() const regex = new RegExp(pattern) for (let i = 0; i < trialCount; i++) { for (let row = 0, rowCount = lines.length; row < rowCount; row++) { @@ -32,11 +37,14 @@ function benchmarkSearch(description, pattern, expectedPosition) { } } } - console.timeEnd(name) - console.log() + const tf2 = performance.now() + console.log(`${name} ${' '.repeat(80-name.length)} ${(tf2-ti2).toFixed(3)} ms`) } benchmarkSearch('simple non-existent pattern', '\t', null) benchmarkSearch('complex non-existent pattern', '123|456|789', null) benchmarkSearch('simple existing pattern', 'jkl', {start: {row: 0, column: 12}, end: {row: 0, column: 15}}) -benchmarkSearch('complex existing pattern', 'j\\w+', {start: {row: 0, column: 12}, end: {row: 0, column: 15}}) \ No newline at end of file +benchmarkSearch('complex existing pattern', 'j\\w+', {start: {row: 0, column: 12}, end: {row: 0, column: 15}}) + + +console.log('\n text-buffer finished \n') \ No newline at end of file From 0e9523a0bb1e146d13fd45ef3cd46c24ec83a000 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Tue, 8 Dec 2020 17:09:02 -0600 Subject: [PATCH 37/40] use performance in marker-index benchmarks + pretty print --- benchmark/marker-index.benchmark.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/benchmark/marker-index.benchmark.js b/benchmark/marker-index.benchmark.js index c6e2ac20..5a1058c3 100644 --- a/benchmark/marker-index.benchmark.js +++ b/benchmark/marker-index.benchmark.js @@ -1,6 +1,10 @@ 'use strict'; +console.log(' running marker-index tests... \n') + const Random = require('random-seed') +const {performance} = require("perf_hooks") + const {MarkerIndex} = require('..') const {traverse, traversalDistance, compare} = require('../test/js/helpers/point-helpers') @@ -41,12 +45,13 @@ function runBenchmark () { } function profileOperations (name, operations) { - console.time(name) + const ti1 = performance.now() for (let i = 0, n = operations.length; i < n; i++) { const operation = operations[i] markerIndex[operation[0]].apply(markerIndex, operation[1]) } - console.timeEnd(name) + const tf1 = performance.now() + console.log(`${name} ${' '.repeat(80-name.length)} ${(tf1-ti1).toFixed(3)} ms`) } function enqueueSequentialInsert () { @@ -118,3 +123,5 @@ function getSplice () { } runBenchmark() + +console.log(' \n marker-index finished \n') From 92e40de755fc45d1f7133e4eb1424f90cd94084f Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 30 Dec 2020 22:54:28 -0600 Subject: [PATCH 38/40] First make everything a shared_ptr Later we can find the unique ones --- src/core/text-buffer.cc | 66 ++++++++++++++++++++--------------------- src/core/text-buffer.h | 11 +++---- 2 files changed, 39 insertions(+), 38 deletions(-) diff --git a/src/core/text-buffer.cc b/src/core/text-buffer.cc index bacd6619..f58cad73 100644 --- a/src/core/text-buffer.cc +++ b/src/core/text-buffer.cc @@ -7,6 +7,7 @@ #include #include #include +#include using std::equal; using std::move; @@ -14,6 +15,10 @@ using std::pair; using std::string; using std::u16string; using std::vector; +using std::unique_ptr; +using std::make_unique; +using std::shared_ptr; +using std::make_shared; using MatchOptions = Regex::MatchOptions; using MatchResult = Regex::MatchResult; using SubsequenceMatch = TextBuffer::SubsequenceMatch; @@ -23,7 +28,7 @@ uint32_t TextBuffer::MAX_CHUNK_SIZE_TO_COPY = 1024; static Text EMPTY_TEXT; struct TextBuffer::Layer { - Layer *previous_layer; + std::shared_ptr previous_layer; Patch patch; optional text; bool uses_patch; @@ -40,7 +45,7 @@ struct TextBuffer::Layer { size_{this->text->size()}, snapshot_count{0} {} - explicit Layer(Layer *previous_layer) : + explicit Layer(std::shared_ptr previous_layer) : previous_layer{previous_layer}, patch{Patch()}, uses_patch{true}, @@ -52,8 +57,8 @@ struct TextBuffer::Layer { return Point(position.row, position.column - 1); } - bool is_above_layer(const Layer *layer) const { - Layer *predecessor = previous_layer; + bool is_above_layer(const shared_ptr layer) const { + auto predecessor = previous_layer; while (predecessor != nullptr) { if (predecessor == layer) return true; predecessor = predecessor->previous_layer; @@ -637,7 +642,7 @@ struct TextBuffer::Layer { return matches; } - bool is_modified(const Layer *base_layer) { + bool is_modified(const shared_ptr base_layer) { if (size() != base_layer->size()) return true; bool result = false; @@ -671,18 +676,17 @@ struct TextBuffer::Layer { }; TextBuffer::TextBuffer(u16string &&text) : - base_layer{new Layer(move(text))}, + base_layer{make_shared(Layer(move(text)))}, top_layer{base_layer} {} TextBuffer::TextBuffer() : - base_layer{new Layer(Text{})}, + base_layer{make_shared(Layer(Text{}))}, top_layer{base_layer} {} TextBuffer::~TextBuffer() { - Layer *layer = top_layer; + auto layer = top_layer; while (layer != nullptr) { - Layer *previous_layer = layer->previous_layer; - delete layer; + auto previous_layer = layer->previous_layer; layer = previous_layer; } } @@ -709,8 +713,7 @@ void TextBuffer::reset(Text &&new_base_text) { layer = top_layer->previous_layer; while (layer != nullptr) { - Layer *previous_layer = layer->previous_layer; - delete layer; + auto previous_layer = layer->previous_layer; layer = previous_layer; } @@ -723,10 +726,10 @@ void TextBuffer::reset(Text &&new_base_text) { top_layer->previous_layer = nullptr; } -Patch TextBuffer::get_inverted_changes(const Snapshot *snapshot) const { - vector patches; - Layer *layer = top_layer; - while (layer != &snapshot->base_layer) { +Patch TextBuffer::get_inverted_changes(const shared_ptr snapshot) const { + vector patches; + auto layer = top_layer; + while (layer.get() != &snapshot->base_layer) { patches.insert(patches.begin(), &layer->patch); layer = layer->previous_layer; } @@ -767,7 +770,7 @@ void TextBuffer::serialize_changes(Serializer &serializer) { } vector patches; - Layer *layer = top_layer; + auto layer = top_layer; while (layer != base_layer) { patches.insert(patches.begin(), &layer->patch); layer = layer->previous_layer; @@ -784,7 +787,7 @@ void TextBuffer::serialize_changes(Serializer &serializer) { bool TextBuffer::deserialize_changes(Deserializer &deserializer) { if (top_layer != base_layer || (base_layer->previous_layer != nullptr)) return false; - top_layer = new Layer(base_layer); + top_layer = make_shared(Layer(base_layer)); top_layer->size_ = deserializer.read(); top_layer->extent_ = Point(deserializer); top_layer->patch = Patch(deserializer); @@ -891,7 +894,7 @@ void TextBuffer::set_text(const u16string &new_text) { void TextBuffer::set_text_in_range(Range old_range, u16string &&string) { if (top_layer == base_layer || top_layer->snapshot_count > 0) { - top_layer = new Layer(top_layer); + top_layer = make_shared(Layer(top_layer)); } auto start = clip_position(old_range.start); @@ -968,13 +971,13 @@ bool TextBuffer::has_astral() { return top_layer->has_astral(); } -bool TextBuffer::is_modified(const Snapshot *snapshot) const { - return top_layer->is_modified(&snapshot->base_layer); +bool TextBuffer::is_modified(const shared_ptr snapshot) const { + return top_layer->is_modified(make_shared(snapshot->base_layer)); } string TextBuffer::get_dot_graph() const { - Layer *layer = top_layer; - vector layers; + auto layer = top_layer; + vector> layers; while (layer != nullptr) { layers.push_back(layer); layer = layer->previous_layer; @@ -998,7 +1001,7 @@ string TextBuffer::get_dot_graph() const { size_t TextBuffer::layer_count() const { size_t result = 1; - const Layer *layer = top_layer; + auto layer = top_layer; while (layer->previous_layer != nullptr) { result++; layer = layer->previous_layer; @@ -1075,7 +1078,8 @@ TextBuffer::Snapshot::Snapshot(TextBuffer &buffer, TextBuffer::Layer &layer, void TextBuffer::Snapshot::flush_preceding_changes() { if (!layer.text) { layer.text = Text{text()}; - if (layer.is_above_layer(buffer.base_layer)) buffer.base_layer = &layer; + if (layer.is_above_layer(buffer.base_layer)) + buffer.base_layer = std::make_shared(layer); buffer.consolidate_layers(); } } @@ -1090,8 +1094,8 @@ TextBuffer::Snapshot::~Snapshot() { } void TextBuffer::consolidate_layers() { - Layer *layer = top_layer; - vector mutable_layers; + auto layer = top_layer; + vector> mutable_layers; bool needed_by_layer_above = false; while (layer != nullptr) { @@ -1116,7 +1120,7 @@ void TextBuffer::consolidate_layers() { squash_layers(mutable_layers); } -void TextBuffer::squash_layers(const vector &layers) { +void TextBuffer::squash_layers(const vector> &layers) { size_t layer_index = 0; size_t layer_count = layers.size(); if (layer_count < 2) return; @@ -1147,7 +1151,7 @@ void TextBuffer::squash_layers(const vector &layers) { // If there is another layer below these layers, combine their patches into // into one. Otherwise, this is the new base layer, so we don't need a patch. Patch patch; - Layer *previous_layer = layers.back()->previous_layer; + auto previous_layer = layers.back()->previous_layer; if (previous_layer != nullptr) { layer_index = layer_count - 1; @@ -1166,8 +1170,4 @@ void TextBuffer::squash_layers(const vector &layers) { layers[0]->previous_layer = previous_layer; layers[0]->text = move(text); layers[0]->patch = move(patch); - - for (layer_index = 1; layer_index < layer_count; layer_index++) { - delete layers[layer_index]; - } } diff --git a/src/core/text-buffer.h b/src/core/text-buffer.h index 30dc1f95..f708c655 100644 --- a/src/core/text-buffer.h +++ b/src/core/text-buffer.h @@ -9,12 +9,13 @@ #include "range.h" #include "regex.h" #include "marker-index.h" +#include class TextBuffer { struct Layer; - Layer *base_layer; - Layer *top_layer; - void squash_layers(const std::vector &); + std::shared_ptr base_layer; + std::shared_ptr top_layer; + void squash_layers(const std::vector> &); void consolidate_layers(); public: @@ -95,8 +96,8 @@ class TextBuffer { friend class Snapshot; Snapshot *create_snapshot(); - bool is_modified(const Snapshot *) const; - Patch get_inverted_changes(const Snapshot *) const; + bool is_modified(const std::shared_ptr ) const; + Patch get_inverted_changes(const std::shared_ptr) const; size_t layer_count() const; std::string get_dot_graph() const; From 0698755e4f2a0c4f837ee5f722f77261db3e0412 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Thu, 7 Jan 2021 23:59:49 -0600 Subject: [PATCH 39/40] Patch missing methods --- src/core/patch.cc | 14 ++++++++++++++ src/core/patch.h | 2 ++ 2 files changed, 16 insertions(+) diff --git a/src/core/patch.cc b/src/core/patch.cc index f0923562..a21ea3c4 100644 --- a/src/core/patch.cc +++ b/src/core/patch.cc @@ -319,6 +319,11 @@ struct Patch::NewCoordinates { Patch::Patch(bool merges_adjacent_changes) : root{nullptr}, change_count{0}, merges_adjacent_changes{merges_adjacent_changes} {} +Patch::Patch(Patch &other) + : root{nullptr}, change_count{other.change_count}, merges_adjacent_changes{other.merges_adjacent_changes} { + *this = other; +} + Patch::Patch(Patch &&other) : root{nullptr}, change_count{other.change_count}, merges_adjacent_changes{other.merges_adjacent_changes} { @@ -378,6 +383,15 @@ Patch::Patch(Deserializer &input) : } } +Patch &Patch::operator=(Patch &other) { + std::swap(root, other.root); + std::swap(left_ancestor_stack, other.left_ancestor_stack); + std::swap(node_stack, other.node_stack); + std::swap(change_count, other.change_count); + merges_adjacent_changes = other.merges_adjacent_changes; + return *this; +} + Patch &Patch::operator=(Patch &&other) { std::swap(root, other.root); std::swap(left_ancestor_stack, other.left_ancestor_stack); diff --git a/src/core/patch.h b/src/core/patch.h index ecfc7c8c..768f2d51 100644 --- a/src/core/patch.h +++ b/src/core/patch.h @@ -36,7 +36,9 @@ class Patch { }; // Construction and destruction + Patch(Patch &); Patch(Patch &&); + Patch &Patch::operator=(Patch &other); Patch &operator=(Patch &&); explicit Patch(bool merges_adjacent_changes = true); explicit Patch(Deserializer &input); From f18a33ba3e28383a7273a61e966e8005eb697013 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Fri, 8 Jan 2021 00:13:35 -0600 Subject: [PATCH 40/40] text-buffer-wrapper --- src/bindings/text-buffer-wrapper.cc | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/bindings/text-buffer-wrapper.cc b/src/bindings/text-buffer-wrapper.cc index 41a7ff7b..765e8810 100644 --- a/src/bindings/text-buffer-wrapper.cc +++ b/src/bindings/text-buffer-wrapper.cc @@ -425,7 +425,6 @@ class TextBufferSearcher : public Nan::AsyncWorker { } void HandleOKCallback() { - delete snapshot; Local argv[] = {Nan::Null(), encode_ranges(matches)}; callback->Call(2, argv, async_resource); } @@ -586,7 +585,6 @@ void TextBufferWrapper::find_words_with_subsequence_in_range(const Nan::Function void CancelIfQueued() { int lock_status = uv_rwlock_trywrlock(&snapshot_lock); if (lock_status == 0) { - delete snapshot; snapshot = nullptr; uv_rwlock_wrunlock(&snapshot_lock); } @@ -599,7 +597,6 @@ void TextBufferWrapper::find_words_with_subsequence_in_range(const Nan::Function return; } - delete snapshot; auto text_buffer_wrapper = Nan::ObjectWrap::Unwrap(Nan::New(buffer)); text_buffer_wrapper->outstanding_workers.erase(this); @@ -730,7 +727,7 @@ class Loader { Nan::Callback *progress_callback; Nan::AsyncResource *async_resource; TextBuffer *buffer; - TextBuffer::Snapshot *snapshot; + std::shared_ptr snapshot; string file_name; string encoding_name; optional loaded_text; @@ -778,17 +775,14 @@ class Loader { pair, Local> Finish(Nan::AsyncResource* caller_async_resource = nullptr) { if (error) { - delete snapshot; return {error_to_js(*error, encoding_name, file_name), Nan::Undefined()}; } if (cancelled || (!force && buffer->is_modified())) { - delete snapshot; return {Nan::Null(), Nan::Null()}; } Patch inverted_changes = buffer->get_inverted_changes(snapshot); - delete snapshot; if (compute_patch && inverted_changes.get_change_count() > 0) { inverted_changes.combine(patch); @@ -1004,7 +998,6 @@ class BaseTextComparisonWorker : public Nan::AsyncWorker { } void HandleOKCallback() { - delete snapshot; if (error) { Local argv[] = {error_to_js(*error, encoding_name, file_name)}; callback->Call(1, argv, async_resource); @@ -1090,11 +1083,9 @@ class SaveWorker : public Nan::AsyncWorker { Local Finish() { if (error) { - delete snapshot; return error_to_js(*error, encoding_name, file_name); } else { snapshot->flush_preceding_changes(); - delete snapshot; return Nan::Null(); } }