diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 00000000..ae1f79b7 --- /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, -bugprone-reserved-identifier, -cert-dcl37-c, -cert-dcl51-cpp" +HeaderFilterRegex: ".*" +FormatStyle: none 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/.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 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: diff --git a/benchmark/large-text-buffer.benchmark.js b/benchmark/large-text-buffer.benchmark.js index 95106bb3..1fca4037 100644 --- a/benchmark/large-text-buffer.benchmark.js +++ b/benchmark/large-text-buffer.benchmark.js @@ -1,56 +1,53 @@ -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') +const {performance} = require("perf_hooks") + +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` - -getText().then(txt => { +getText().then(async (txt) => { const buffer = new TextBuffer() - console.log('running findWordsWithSubsequence tests...') + console.log('\n running large-text-buffer tests... \n') - const sizes = [['10b', 10], ['100b', 100], ['1kb', 1000], ['1MB', 1000000], ['51MB', 100000000]] + const sizes = [ ['100b', 100], ['1kb', 1000], ['1MB', 1000000], ['51MB', 100000000], ['119MB', txt.length]] - const test = size => { - const _timer = timer(size[0]) - buffer.setText(txt.slice(0, size[1])) - console.time(_timer) - return buffer.findWordsWithSubsequence('cat', '', 100).then(sugs => { - console.timeEnd(_timer) - }) + const test = async (word, size) => { + const ti2 = performance.now() + await buffer.findWordsWithSubsequence(word, '', 100) + 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) { - return sizes.reduce((promise, size) => { - return promise.then(() => test(size)) - }, Promise.resolve()) + 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('finished') + console.log(' large-text-buffer finished \n') }) 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') diff --git a/benchmark/text-buffer.benchmark.js b/benchmark/text-buffer.benchmark.js index 36152b1f..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.searchSync(pattern), expectedPosition) + 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 diff --git a/binding.gyp b/binding.gyp index 249e9e82..85981bbb 100644 --- a/binding.gyp +++ b/binding.gyp @@ -67,7 +67,9 @@ ], "variables": { - "tests": 0 + "tests": 0, + "STANDARD": 17, + "MACOSX_DEPLOYMENT_TARGET": "10.8" }, "conditions": [ @@ -100,20 +102,19 @@ "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)', } }] ] }] }] ], - "target_defaults": { - "cflags_cc": ["-std=c++11"], + "cflags_cc": [ "-std=c++<(STANDARD)" ], "conditions": [ ['OS=="mac"', { "xcode_settings": { @@ -129,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 } diff --git a/package.json b/package.json index 9ea8aa1b..740d36ac 100644 --- a/package.json +++ b/package.json @@ -12,9 +12,12 @@ "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/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" + "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" }, "repository": { "type": "git", @@ -35,11 +38,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": [ 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/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..00727f5d 100644 --- a/src/bindings/marker-index-wrapper.cc +++ b/src/bindings/marker-index-wrapper.cc @@ -1,9 +1,10 @@ #include "marker-index-wrapper.h" #include #include "marker-index.h" -#include "nan.h" +#include #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..c3f5eea9 100644 --- a/src/bindings/marker-index-wrapper.h +++ b/src/bindings/marker-index-wrapper.h @@ -1,6 +1,7 @@ -#include "nan.h" +#include #include "marker-index.h" -#include "optional.h" +#include +using std::optional; #include "range.h" class MarkerIndexWrapper : public Nan::ObjectWrap { @@ -36,6 +37,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/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 cc063be9..c0c98b30 100644 --- a/src/bindings/number-conversion.h +++ b/src/bindings/number-conversion.h @@ -1,8 +1,9 @@ #ifndef SUPERSTRING_NUMBER_CONVERSION_H #define SUPERSTRING_NUMBER_CONVERSION_H -#include "nan.h" -#include "optional.h" +#include +#include +using std::optional; namespace number_conversion { template diff --git a/src/bindings/patch-wrapper.cc b/src/bindings/patch-wrapper.cc index fc610aaf..e8794fcf 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: @@ -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/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.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 fc06b263..42fc7f19 100644 --- a/src/bindings/point-wrapper.h +++ b/src/bindings/point-wrapper.h @@ -1,8 +1,9 @@ #ifndef SUPERSTRING_POINT_WRAPPER_H #define SUPERSTRING_POINT_WRAPPER_H -#include "nan.h" -#include "optional.h" +#include +#include +using std::optional; #include "point.h" class PointWrapper : public Nan::ObjectWrap { @@ -12,7 +13,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.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 de08dd59..6aa035aa 100644 --- a/src/bindings/range-wrapper.h +++ b/src/bindings/range-wrapper.h @@ -1,8 +1,9 @@ #ifndef SUPERSTRING_RANGE_WRAPPER_H #define SUPERSTRING_RANGE_WRAPPER_H -#include "nan.h" -#include "optional.h" +#include +#include +using std::optional; #include "point.h" #include "range.h" @@ -13,7 +14,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/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/string-conversion.h b/src/bindings/string-conversion.h index f8178041..abeeeee7 100644 --- a/src/bindings/string-conversion.h +++ b/src/bindings/string-conversion.h @@ -2,8 +2,9 @@ #define SUPERSTRING_STRING_CONVERSION_H #include -#include "nan.h" -#include "optional.h" +#include +#include +using std::optional; #include "text.h" namespace string_conversion { 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/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.cc b/src/bindings/text-buffer-wrapper.cc index 52e7bded..765e8810 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" @@ -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() { @@ -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, @@ -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); @@ -665,7 +662,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; @@ -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; @@ -742,7 +739,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 +752,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}, @@ -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); @@ -848,13 +842,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 +984,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}, @@ -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); @@ -1050,7 +1043,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}, @@ -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(); } } 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..835a3deb 100644 --- a/src/bindings/text-writer.h +++ b/src/bindings/text-writer.h @@ -1,14 +1,14 @@ #ifndef SUPERSTRING_TEXT_WRITER_H #define SUPERSTRING_TEXT_WRITER_H -#include "nan.h" +#include #include "text.h" #include "encoding-conversion.h" 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/encoding-conversion.cc b/src/core/encoding-conversion.cc index 7259d40d..932d6c96 100644 --- a/src/core/encoding-conversion.cc +++ b/src/core/encoding-conversion.cc @@ -1,16 +1,16 @@ #include "encoding-conversion.h" #include "utf8-conversions.h" #include -#include +#include 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, @@ -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( @@ -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; diff --git a/src/core/encoding-conversion.h b/src/core/encoding-conversion.h index 3b91146a..fe667c3d 100644 --- a/src/core/encoding-conversion.h +++ b/src/core/encoding-conversion.h @@ -1,9 +1,10 @@ #ifndef SUPERSTRING_ENCODING_CONVERSION_H_ #define SUPERSTRING_ENCODING_CONVERSION_H_ -#include "optional.h" +#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..dca69cda 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; @@ -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/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..fc24dab4 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; @@ -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); @@ -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); @@ -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; } @@ -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 { @@ -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; } @@ -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) { 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/optional.h b/src/core/optional.h deleted file mode 100644 index 61280d99..00000000 --- a/src/core/optional.h +++ /dev/null @@ -1,29 +0,0 @@ -#ifndef SUPERSTRING_OPTIONAL_H -#define SUPERSTRING_OPTIONAL_H - -#include - -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 diff --git a/src/core/patch.cc b/src/core/patch.cc index 8702f9b3..a21ea3c4 100644 --- a/src/core/patch.cc +++ b/src/core/patch.cc @@ -1,11 +1,12 @@ #include "patch.h" -#include "optional.h" +#include +using std::optional; #include "text.h" #include "text-slice.h" -#include +#include #include #include -#include +#include #include #include @@ -17,7 +18,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; @@ -36,7 +37,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, @@ -59,7 +60,7 @@ struct Patch::Node { compute_subtree_text_sizes(); } - Node(Deserializer &input) : + explicit Node(Deserializer &input) : left{nullptr}, right{nullptr}, old_extent{input}, @@ -104,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) { @@ -124,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) @@ -232,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 { @@ -241,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 { @@ -263,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"; @@ -318,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} { @@ -377,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); @@ -387,28 +402,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); @@ -428,7 +443,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); @@ -436,11 +451,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); } @@ -452,7 +467,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); @@ -460,11 +475,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); } @@ -482,7 +497,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), @@ -505,13 +520,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; @@ -658,7 +673,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 = @@ -701,7 +716,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 = @@ -711,7 +726,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 = @@ -770,15 +785,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); @@ -786,7 +801,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; } @@ -801,7 +816,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); } @@ -809,7 +824,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 = @@ -827,7 +842,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); @@ -836,7 +851,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) { @@ -874,8 +889,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) { @@ -904,19 +919,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 { @@ -951,7 +966,7 @@ optional Patch::get_bounds() const { if (!root) return optional{}; Node *node = root; - while (node->left) { + while (node->left != nullptr) { node = node->left; } @@ -960,7 +975,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; @@ -1002,7 +1017,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); @@ -1019,7 +1034,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(); @@ -1032,7 +1047,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; @@ -1104,7 +1119,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(); @@ -1136,7 +1151,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 { @@ -1161,7 +1176,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 { @@ -1188,15 +1203,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; @@ -1217,10 +1232,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; @@ -1320,16 +1335,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--; @@ -1861,7 +1876,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"; @@ -1869,7 +1884,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/patch.h b/src/core/patch.h index c59021e5..768f2d51 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" @@ -35,10 +36,12 @@ class Patch { }; // Construction and destruction - Patch(bool merges_adjacent_changes = true); + Patch(Patch &); Patch(Patch &&); - Patch(Deserializer &input); + Patch &Patch::operator=(Patch &other); 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.cc b/src/core/regex.cc index 71629642..c2c89149 100644 --- a/src/core/regex.cc +++ b/src/core/regex.cc @@ -1,6 +1,6 @@ #include "regex.h" -#include -#include "pcre2.h" +#include +#include using std::u16string; using MatchResult = Regex::MatchResult; @@ -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/regex.h b/src/core/regex.h index 10970306..b3b973ef 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; @@ -10,7 +11,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 +25,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.cc b/src/core/text-buffer.cc index c4dd3e8c..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; @@ -32,7 +37,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 +45,7 @@ struct TextBuffer::Layer { size_{this->text->size()}, snapshot_count{0} {} - Layer(Layer *previous_layer) : + explicit Layer(std::shared_ptr previous_layer) : previous_layer{previous_layer}, patch{Patch()}, uses_patch{true}, @@ -52,9 +57,9 @@ struct TextBuffer::Layer { return Point(position.row, position.column - 1); } - bool is_above_layer(const Layer *layer) const { - Layer *predecessor = previous_layer; - while (predecessor) { + 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; } @@ -523,11 +528,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; @@ -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; - while (layer) { - Layer *previous_layer = layer->previous_layer; - delete layer; + auto layer = top_layer; + while (layer != nullptr) { + auto previous_layer = layer->previous_layer; layer = previous_layer; } } @@ -693,7 +697,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,9 +712,8 @@ void TextBuffer::reset(Text &&new_base_text) { } layer = top_layer->previous_layer; - while (layer) { - Layer *previous_layer = layer->previous_layer; - delete layer; + while (layer != nullptr) { + 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; @@ -783,8 +786,8 @@ void TextBuffer::serialize_changes(Serializer &serializer) { } bool TextBuffer::deserialize_changes(Deserializer &deserializer) { - if (top_layer != base_layer || base_layer->previous_layer) return false; - top_layer = new Layer(base_layer); + if (top_layer != base_layer || (base_layer->previous_layer != nullptr)) return false; + 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,14 +971,14 @@ 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; - while (layer) { + auto layer = top_layer; + vector> layers; + while (layer != nullptr) { layers.push_back(layer); layer = layer->previous_layer; } @@ -998,8 +1001,8 @@ 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) { + 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,11 +1094,11 @@ 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) { + while (layer != nullptr) { if (needed_by_layer_above || layer->snapshot_count > 0) { squash_layers(mutable_layers); mutable_layers.clear(); @@ -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,9 +1151,9 @@ 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) { + if (previous_layer != nullptr) { layer_index = layer_count - 1; patch = move(layers[layer_index]->patch); layer_index--; @@ -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 5b2800cb..f708c655 100644 --- a/src/core/text-buffer.h +++ b/src/core/text-buffer.h @@ -9,20 +9,21 @@ #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: 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; @@ -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; 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} {} 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; diff --git a/src/core/text.h b/src/core/text.h index 0326a902..f4e10b03 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; @@ -28,11 +29,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}) {}