ronantakizawa/codereview-bench huggingface dataset:
Repository: sz3/libcimbar
File: src/lib/cimb_translator/GridConf.h (C/C++)

Code before:
			return lrint(54.0 / cell_spacing_x);
		}

		unsigned corner_padding_y() const
		{
			return lrint(54.0 / cell_spacing_y);
		}

		unsigned total_cells() const
		{
			return cells_per_col_x*cells_per_col_y - (corner_padding_x()*corner_padding_y() * 4);
		}

		unsigned capacity(unsigned bitspercell) const
		{
			if (!bitspercell)
				bitspercell = bits_per_cell();
			return total_cells() * bitspercell / 8;
		}

		unsigned fountain_chunks_per_frame(unsigned bitspercell=0) const
		{
			if (!bitspercell)
				bitspercell = bits_per_cell();
			if (legacy_mode or !fountain_chunks_scalar)
				return 10;
			return bitspercell * fountain_chunks_scalar;
		}

		unsigned fountain_chunk_size(unsigned bitspercell=0) const
		{
			// TODO: sanity checks?
			// this should neatly split into fountain_chunks_per_frame() [ex: 10] chunks per frame.
			// the other reasonable settings for fountain_chunks_per_frame are `2` and `5`
			if (!bitspercell)
				bitspercell = bits_per_cell();
			return capacity(bitspercell) * (ecc_block_size-ecc_bytes) / ecc_block_size / fountain_chunks_per_frame(bitspercell);
		}

	};

	struct Conf5x5 : conf
	{
		Conf5x5()
			: conf()
		{
			color_bits = 2;
			symbol_bits = 2;
			ecc_bytes = 40;
			ecc_block_size = 216;
			image_size_x = 988;

Review comment (suggestion):
Formally making negative numbers refer to a constant fountain_chunk_size (for now). This is an impl detail, it's not really important how we represent it. But it's sometimes useful when testing configs to be able to lock the chunk size to a constant value.

Code after:
			return lrint(54.0 / cell_spacing_x);
		}

		unsigned corner_padding_y() const
		{
			return lrint(54.0 / cell_spacing_y);
		}

		unsigned total_cells() const
		{
			return cells_per_col_x*cells_per_col_y - (corner_padding_x()*corner_padding_y() * 4);
		}

		unsigned capacity(unsigned bitspercell) const
		{
			if (!bitspercell)
				bitspercell = bits_per_cell();
			return total_cells() * bitspercell / 8;
		}

		unsigned fountain_chunks_per_frame(unsigned bitspercell=0) const
		{
			if (!bitspercell)
				bitspercell = bits_per_cell();
			if (fountain_chunks_scalar < 0)
				return -fountain_chunks_scalar;
			return bitspercell * fountain_chunks_scalar;
		}

		unsigned fountain_chunk_size(unsigned bitspercell=0) const
		{
			// TODO: sanity checks?
			// this should neatly split into fountain_chunks_per_frame() [ex: 10] chunks per frame.
			// the other reasonable settings for fountain_chunks_per_frame are `2` and `5`
			if (!bitspercell)
				bitspercell = bits_per_cell();
			return capacity(bitspercell) * (ecc_block_size-ecc_bytes) / ecc_block_size / fountain_chunks_per_frame(bitspercell);
		}

	};

	struct Conf5x5 : conf
	{
		Conf5x5()
			: conf()
		{
			color_bits = 2;
			symbol_bits = 2;
			ecc_bytes = 40;
			ecc_block_size = 216;
			image_size_x = 988;
Repository: trueadm/ripple
File: packages/ripple/src/runtime/internal/client/runtime.js (JavaScript)

Code before:
/**
 * @template T
 * @param {Block} block
 * @param {() => T} fn
 * @returns {T}
 */
export function with_scope(block, fn) {
	var previous_scope = active_scope;
	try {
		active_scope = block;
		return fn();
	} finally {
		active_scope = previous_scope;
	}
}

export function scope() {
	return active_scope;
}

export function safe_scope(err = 'Cannot access outside of a component context') {
	if (active_scope === null) {
		throw new Error(err);
	}

	return scope();
}

export function push_component() {
	var component = {
		c: null,
		e: null,
		m: false,
		p: active_component,
	};
	active_component = component;
}

export function pop_component() {
	var component = /** @type {Component} */ (active_component);
	component.m = true;
	var effects = component.e;
	if (effects !== null) {
		var length = effects.length;
		for (var i = 0; i < length; i++) {
			var { b: block, fn, r: reaction } = effects[i];
			var previous_block = active_block;
			var previous_reaction = active_reaction;

			try {
				active_block = block;

Review comment (suggestion):
this can just return `active_scope` right? Also it's missing jsdoc types

Code after:
/**
 * @template T
 * @param {Block} block
 * @param {() => T} fn
 * @returns {T}
 */
export function with_scope(block, fn) {
	var previous_scope = active_scope;
	try {
		active_scope = block;
		return fn();
	} finally {
		active_scope = previous_scope;
	}
}

/**
 * @returns {Block | null}
 */
export function scope() {
	return active_scope;
}

/**
 * @param {string} [err]
 * @returns {Block | never}
 */
export function safe_scope(err = 'Cannot access outside of a component context') {
	if (active_scope === null) {
		throw new Error(err);
	}

	return /** @type {Block} */ (active_scope);
}

export function push_component() {
	var component = {
		c: null,
		e: null,
		m: false,
		p: active_component,
	};
	active_component = component;
}

export function pop_component() {
	var component = /** @type {Component} */ (active_component);
	component.m = true;
	var effects = component.e;
	if (effects !== null) {
		var length = effects.length;
Repository: trueadm/ripple
File: packages/ripple/src/compiler/phases/3-transform/index.js (JavaScript)

Code before:
				name === 'forEach' ||
				name === 'join' ||
				name === 'includes' ||
				name === 'indexOf' ||
				name === 'lastIndexOf' ||
				name === 'filter' ||
				name === 'every' ||
				name === 'some' ||
				name === 'toSpliced' ||
				name === 'toSorted' ||
				name === 'toString' ||
				name === 'values' ||
				name === 'entries'
			) {
				return b.call(
					'$.with_scope',
					b.id('__block'),
					b.thunk(
						b.call(
							'$.array_' + name,
							context.visit(callee.object),
							...node.arguments.map((arg) => context.visit(arg)),
						),
					),
				);
			}
		}

		return b.call(
			'$.with_scope',
			b.id('__block'),
			b.thunk({
				...node,
				callee: context.visit(callee),
				arguments: node.arguments.map((arg) => context.visit(arg)),
			}),
		);
	},

	TSTypeAliasDeclaration(_, context) {
		if (!context.state.to_ts) {
			return b.empty;
		}
		context.next();
	},

	TSInterfaceDeclaration(_, context) {
		if (!context.state.to_ts) {
			return b.empty;
		}
		context.next();

Review comment (suggestion):
we should probably also do the same with tuple arrays (just wrap in `call_property`) and handle this in runtime.  I was thinking to attach another unique symbol or just one that's unique on tuples to identify them in runtime and there just apply whatever methods are being called on them.

But better to wait to decide what we'll do with our recent proposals.

Code after:
					name === 'join' ||
					name === 'includes' ||
					name === 'indexOf' ||
					name === 'lastIndexOf' ||
					name === 'filter' ||
					name === 'every' ||
					name === 'some' ||
					name === 'toSpliced' ||
					name === 'toSorted' ||
					name === 'toString' ||
					name === 'values' ||
					name === 'entries'
				) {
					return b.call(
						'$.with_scope',
						b.id('__block'),
						b.thunk(
							b.call(
								'$.array_' + name,
								context.visit(callee.object),
								...node.arguments.map((arg) => context.visit(arg)),
							),
						),
					);
				}
			}

			if (callee.computed) {
				return b.call(
					'$.with_scope',
					b.id('__block'),
					b.thunk(
						b.call(
							'$.call_property',
							context.visit(callee.object),
							context.visit(property),
							callee.optional ? b.true : undefined,
							node.optional ? b.true : undefined,
							...node.arguments.map((arg) => context.visit(arg)),
						)
					)
				);
			}
		}

		return b.call(
			'$.with_scope',
			b.id('__block'),
			b.thunk({
				...node,
				callee: context.visit(callee),
Repository: trueadm/ripple
File: packages/ripple/src/runtime/internal/client/runtime.js (JavaScript)

Code before:
  var previous_queued_root_blocks = queued_root_blocks;
  queued_root_blocks = [];
  flush_queued_root_blocks(previous_queued_root_blocks);

  if (!is_micro_task_queued) {
    flush_count = 0;
  }
  old_values.clear();
}

/**
 * @param { (() => void) } [fn]
 */
export function queue_microtask(fn) {
  if (!is_micro_task_queued) {
    is_micro_task_queued = true;
    queueMicrotask(flush_microtasks);
  }
  if (fn !== undefined) {
    queued_microtasks.push(fn);
  }
}

/**
 * @param {Block} block
 */
export function schedule_update(block) {
  if (scheduler_mode === FLUSH_MICROTASK) {
    queue_microtask();
  }
  let current = block;

  while (current !== null) {
    var flags = current.f;
    if ((flags & CONTAINS_UPDATE) !== 0) return;
    current.f ^= CONTAINS_UPDATE;
    if ((flags & ROOT_BLOCK) !== 0) {
      break;
    }
    current = /** @type {Block} */ (current.p);
  }

  queued_root_blocks.push(current);
}

/**
 * @param {Tracked} tracked
 */
function register_dependency(tracked) {
  var dependency = active_dependency;


Review comment (suggestion):
```suggestion
/**
 * @returns {Promise<void>}
 */
export async function tick() {
	return new Promise((f) => requestAnimationFrame(() => f()));
	flushSync();
}
```

since we have try / pending and allow awaits in components

Code after:
		try {
			if ((flags & (PAUSED | DESTROYED)) === 0 && is_block_dirty(effect)) {
				run_block(effect);
			}
		} catch (error) {
			handle_error(error, effect);
		}
	}
}

/**
 * @param {Block[]} root_blocks
 */
function flush_queued_root_blocks(root_blocks) {
	for (let i = 0; i < root_blocks.length; i++) {
		flush_updates(root_blocks[i]);
	}
}

// https://github.com/sveltejs/svelte/blob/ded13b825d7efcdf064fd65a5aa9e7e61293a48b/packages/svelte/src/internal/client/runtime.js#L501
/**
 * @returns {Promise<void>}
 */
export async function tick() {
  return new Promise((f) => requestAnimationFrame(() => f()));
}

/**
 * @returns {void}
 */
function flush_microtasks() {
	is_micro_task_queued = false;

	if (queued_microtasks.length > 0) {
		var microtasks = queued_microtasks;
		queued_microtasks = [];
		for (var i = 0; i < microtasks.length; i++) {
			microtasks[i]();
		}
	}

	if (flush_count > 1001) {
		return;
	}
	var previous_queued_root_blocks = queued_root_blocks;
	queued_root_blocks = [];
	flush_queued_root_blocks(previous_queued_root_blocks);

	if (!is_micro_task_queued) {
		flush_count = 0;
	}
Repository: trueadm/ripple
File: packages/ripple/src/runtime/array.js (JavaScript)

Code before:
/**
 * @template T
 * @param {ArrayLike<T> | Iterable<T>} arrayLike
 * @param {(v: T, k: number) => any | undefined} [mapFn]
 * @param {any} [thisArg]
 * @returns {Promise<TrackedArray<T>>}
 */
TrackedArray.fromAsync = async function (arrayLike, mapFn, thisArg) {
  var block = safe_scope();
  var elements = mapFn
    ? await Array.fromAsync(arrayLike, mapFn, thisArg)
    : await Array.fromAsync(arrayLike);
  return proxy(elements, block, true);
};

/**
 * @template T
 * @param {Iterable<T>} elements
 * @param {Block} block
 * @param {boolean} is_from_static
 * @returns {TrackedArray<T>}
 */
function proxy(elements, block, is_from_static = false) {
  var arr;
  var first;

  if (
    is_from_static &&
    (first = get_first_if_length(/** @type {Array<T>} */ (elements))) !== undefined
  ) {
    arr = new Array();
    arr[0] = first;
  } else {
    arr = new Array(...elements);
  }

  var tracked_elements = new Map();
  var tracked_len = tracked(arr.length, block);
  tracked_elements.set('length', tracked_len);

  return new Proxy(arr, {
    get(target, prop, receiver) {
      var t = tracked_elements.get(prop);
      var exists = prop in target;

      if (t === undefined && (!exists || get_descriptor(target, prop)?.writable)) {
        t = tracked(exists ? target[prop] : UNINITIALIZED, block);
        tracked_elements.set(prop, t);
      }

      if (t !== undefined) {

Review comment (suggestion):
```diff
-- from_static: from_static = false
++ from_static = false
```

Code after:
/**
 * @template T
 * @param {ArrayLike<T> | Iterable<T>} arrayLike
 * @param {(v: T, k: number) => any | undefined} [mapFn]
 * @param {any} [thisArg]
 * @returns {Promise<TrackedArray<T>>}
 */
TrackedArray.fromAsync = async function (arrayLike, mapFn, thisArg) {
  var block = safe_scope();
  var elements = mapFn
    ? await Array.fromAsync(arrayLike, mapFn, thisArg)
    : await Array.fromAsync(arrayLike);
  return proxy({ elements, block, from_static: true });
};

/**
 * @template T
 * @param {{
 *  elements: Iterable<T>,
 *  block: Block,
 *  from_static?: boolean,
 *  use_array?: boolean
 * }} params
 * @returns {TrackedArray<T>}
 */
function proxy({ elements, block, from_static: from_static = false, use_array = false }) {
  var arr;
  var first;

  if (
    from_static &&
    (first = get_first_if_length(/** @type {Array<T>} */ (elements))) !== undefined
  ) {
    arr = new Array();
    arr[0] = first;
  } else if (use_array) {
    arr = elements;
  } else {
    arr = new Array(...elements);
  }

  var tracked_elements = new Map();
  var tracked_len = tracked(arr.length, block);
  tracked_elements.set('length', tracked_len);

  return new Proxy(arr, {
    get(target, prop, receiver) {
      var t = tracked_elements.get(prop);
      var exists = prop in target;

      if (t === undefined && (!exists || get_descriptor(target, prop)?.writable)) {
Repository: trueadm/ripple
File: packages/ripple/src/runtime/internal/client/template.js (JavaScript)

Code before:
    if (node === undefined) {
      node = create_fragment_from_html(has_start ? content : '<!>' + content);
      if (!is_fragment) node = first_child(node);
    }

    var clone =
      use_import_node || is_firefox ? document.importNode(node, true) : node.cloneNode(true);

    if (is_fragment) {
      var start = first_child(clone);
      var end = clone.lastChild;

      assign_nodes(start, end);
    } else {
      assign_nodes(clone, clone);
    }

    return clone;
  };
}

export function append(anchor, dom) {
  anchor.before(/** @type {Node} */ (dom));
}

const SVG_ELEMENTS = new Set([
  'svg',
  'g',
  'defs',
  'symbol',
  'use',
  'switch',
  'rect',
  'circle',
  'ellipse',
  'line',
  'polyline',
  'polygon',
  'path',
  'text',
  'tspan',
  'textPath',
  'linearGradient',
  'radialGradient',
  'pattern',
  'stop',
  'clipPath',
  'mask',
  'filter',
  'feBlend',
  'feColorMatrix',

Review comment (suggestion):
We don't want to be doing this during runtime, this should be something the compiler does using namespaces. See how Svelte does this:

https://github.com/sveltejs/svelte/blob/main/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js#L292

https://github.com/sveltejs/svelte/blob/main/packages/svelte/src/internal/client/dom/template.js#L91






Code after:
function create_fragment_from_html(html, use_svg_namespace = false, use_mathml_namespace = false) {
  if (use_svg_namespace) {
    return from_namespace(html, 'svg');
  }
  if (use_mathml_namespace) {
    return from_namespace(html, 'math');
  }
  var elem = document.createElement('template');
  elem.innerHTML = html;
  return elem.content;
}

/**
 * Creates a template node or fragment from content and flags.
 * @param {string} content - The template content.
 * @param {number} flags - Flags for template type.
 * @returns {Node}
 */
export function template(content, flags) {
  var is_fragment = (flags & TEMPLATE_FRAGMENT) !== 0;
  var use_import_node = (flags & TEMPLATE_USE_IMPORT_NODE) !== 0;
  var use_svg_namespace = (flags & TEMPLATE_SVG_NAMESPACE) !== 0;
  var use_mathml_namespace = (flags & TEMPLATE_MATHML_NAMESPACE) !== 0;
  var node;
  var has_start = !content.startsWith('<!>');

  return () => {
    if (node === undefined) {
      node = create_fragment_from_html(
        has_start ? content : '<!>' + content,
        use_svg_namespace,
        use_mathml_namespace,
      );
      if (!is_fragment) node = first_child(node);
    }

    var clone =
      use_import_node || is_firefox ? document.importNode(node, true) : node.cloneNode(true);

    if (is_fragment) {
      var start = first_child(clone);
      var end = clone.lastChild;

      assign_nodes(start, end);
    } else {
      assign_nodes(clone, clone);
    }

    return clone;
  };
}
Repository: trueadm/ripple
File: packages/ripple/src/runtime/internal/client/template.js (JavaScript)

Code before:
      var start = first_child(clone);
      var end = clone.lastChild;

      assign_nodes(start, end);
    } else {
      assign_nodes(clone, clone);
    }

    return clone;
  };
}

/**
 * Appends a DOM node before the anchor node.
 * @param {Node} anchor - The anchor node.
 * @param {Node} dom - The DOM node to append.
 */
export function append(anchor, dom) {
  anchor.before(/** @type {Node} */ (dom));
}

const SVG_NAMESPACE = 'http://www.w3.org/2000/svg';
const MATHML_NAMESPACE = 'http://www.w3.org/1998/Math/MathML';

function create_svg_fragment_from_html(html) {
  var svgContainer = document.createElementNS(SVG_NAMESPACE, 'svg');
  svgContainer.innerHTML = html;

  var fragment = document.createDocumentFragment();
  while (svgContainer.firstChild) {
    fragment.appendChild(svgContainer.firstChild);
  }
  return fragment;
}

function create_mathml_fragment_from_html(html) {
  var mathContainer = document.createElementNS(MATHML_NAMESPACE, 'math');
  mathContainer.innerHTML = html;

  var fragment = document.createDocumentFragment();
  while (mathContainer.firstChild) {
    fragment.appendChild(mathContainer.firstChild);
  }
  return fragment;
}


Review comment (refactor):
We can simplify this by simply wrapping the template in `svg` or `math`, this will force the namespace to be correct instead of doing it this way. See https://github.com/sveltejs/svelte/blob/main/packages/svelte/src/internal/client/dom/template.js#L99

Code after:
      var start = first_child(clone);
      var end = clone.lastChild;

      assign_nodes(start, end);
    } else {
      assign_nodes(clone, clone);
    }

    return clone;
  };
}

/**
 * Appends a DOM node before the anchor node.
 * @param {Node} anchor - The anchor node.
 * @param {Node} dom - The DOM node to append.
 */
export function append(anchor, dom) {
  anchor.before(/** @type {Node} */ (dom));
}

/**
 * Create fragment with proper namespace using Svelte's wrapping approach
 * @param {string} content
 * @param {'svg' | 'math'} ns
 * @returns {DocumentFragment}
 */
function from_namespace(content, ns = 'svg') {
  var wrapped = `<${ns}>${content}</${ns}>`;

  var elem = document.createElement('template');
  elem.innerHTML = wrapped;
  var fragment = elem.content;

  var root = /** @type {Element} */ (first_child(fragment));
  var result = document.createDocumentFragment();

  while (first_child(root)) {
    result.appendChild(/** @type {Node} */ (first_child(root)));
  }

  return result;
}
Repository: trueadm/ripple
File: packages/ripple/src/runtime/internal/client/portal.js (JavaScript)

Code before:
  let children = UNINITIALIZED;
  /** @type {Block | null} */
  var b = null;
  /** @type {Text | null} */
  var anchor = null;
  /** @type {Node | null} */
  var domStart = null;
  /** @type {Node | null} */
  var domEnd = null;

  render(() => {
    if (target === (target = props.target)) return;
    if (children === (children = props.children)) return;

    if (b !== null) {
      destroy_block(b);
    }

    domStart = domEnd = null;

    anchor = create_text();
    /** @type {Element} */ (target).append(anchor);

    const cleanup_events = handle_root_events(/** @type {Element} */ (target));

    b = branch(() => /** @type {(anchor: Node) => void} */ (children)(/** @type {Text} */ (anchor)));

    if (b && b.s) {
      domStart = b.s.start;
      domEnd = b.s.end;
    }

    return () => {
      cleanup_events();
      /** @type {Text} */ (anchor).remove();
      if(domStart !== null && domEnd !== null) {
        remove_block_dom(domStart, domEnd);
      }
    };
  });
}


Review comment (suggestion):
unrelated but these inline types here make it impossible to read the code, we should just `// @ts-ignore` it.

Code after:
	/** @type {((anchor: Node, props: {}, block: Block) => void) | symbol} */
	let children = UNINITIALIZED;
	/** @type {Block | null} */
	var b = null;
	/** @type {Text | null} */
	var anchor = null;
	/** @type {Node | null} */
	var dom_start = null;
	/** @type {Node | null} */
	var dom_end = null;

	render(() => {
		if (target === (target = props.target)) return;
		if (children === (children = props.children)) return;

		if (b !== null) {
			destroy_block(b);
		}

		if (anchor !== null) {
			anchor.remove();
		}

		dom_start = dom_end = null;

		anchor = create_text();
		/** @type {Element} */ (target).append(anchor);

		const cleanup_events = handle_root_events(/** @type {Element} */ (target));

		var block = /** @type {Block} */ (active_block);

		b = branch(() => {
			if (typeof children === 'function') {
				children(/** @type {Text} */ (anchor), {}, block);
			}
		});

		dom_start = b?.s?.start;
		dom_end = b?.s?.end;

		return () => {
			cleanup_events();
			/** @type {Text} */ (anchor).remove();
			if (dom_start && dom_end) {
				remove_block_dom(dom_start, dom_end);
			}
		};
	});
}
Repository: trueadm/ripple
File: packages/ripple/src/runtime/internal/client/portal.js (JavaScript)

Code before:
  /** @type {Node | null} */
  var domStart = null;
  /** @type {Node | null} */
  var domEnd = null;

  render(() => {
    if (target === (target = props.target)) return;
    if (children === (children = props.children)) return;

    if (b !== null) {
      destroy_block(b);
    }

    domStart = domEnd = null;

    anchor = create_text();
    /** @type {Element} */ (target).append(anchor);

    const cleanup_events = handle_root_events(/** @type {Element} */ (target));

    b = branch(() => /** @type {(anchor: Node) => void} */ (children)(/** @type {Text} */ (anchor)));

    if (b && b.s) {
      domStart = b.s.start;
      domEnd = b.s.end;
    }

    return () => {
      cleanup_events();
      /** @type {Text} */ (anchor).remove();
      if(domStart !== null && domEnd !== null) {
        remove_block_dom(domStart, domEnd);
      }
    };
  });
}


Review comment (suggestion):
```suggestion
    domStart = b?.s?.start;
    domEnd = b?.s?.end;
```

Code after:
	var anchor = null;
	/** @type {Node | null} */
	var dom_start = null;
	/** @type {Node | null} */
	var dom_end = null;

	render(() => {
		if (target === (target = props.target)) return;
		if (children === (children = props.children)) return;

		if (b !== null) {
			destroy_block(b);
		}

		if (anchor !== null) {
			anchor.remove();
		}

		dom_start = dom_end = null;

		anchor = create_text();
		/** @type {Element} */ (target).append(anchor);

		const cleanup_events = handle_root_events(/** @type {Element} */ (target));

		var block = /** @type {Block} */ (active_block);

		b = branch(() => {
			if (typeof children === 'function') {
				children(/** @type {Text} */ (anchor), {}, block);
			}
		});

		dom_start = b?.s?.start;
		dom_end = b?.s?.end;

		return () => {
			cleanup_events();
			/** @type {Text} */ (anchor).remove();
			if (dom_start && dom_end) {
				remove_block_dom(dom_start, dom_end);
			}
		};
	});
}