[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[groff] 09/32: [troff]: Refactor and fix code style nits.
From: |
G. Branden Robinson |
Subject: |
[groff] 09/32: [troff]: Refactor and fix code style nits. |
Date: |
Thu, 6 Oct 2022 09:11:22 -0400 (EDT) |
gbranden pushed a commit to branch master
in repository groff.
commit 515b35b9b1d3e82b4eeeccc585139b6e08936057
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Tue Oct 4 01:52:33 2022 -0500
[troff]: Refactor and fix code style nits.
* src/roff/troff/number.cpp: Refactor and fix code style nits. Boolify
and rename static (local) functions.
- parse_expr -> is_valid_expression
- start_number -> is_valid_expression_start
- parse_term -> is_valid_term
Rename preprocessor macro `SCALE_INDICATOR_CHARS` to `SCALING_UNITS`.
(is_valid_expression, is_valid_term): Rename parameters and demote
them from `int` to `bool`.
- scaling_indicator -> scaling_unit (no demotion)
- parenthesised -> is_parenthesized
- rigid -> is_mandatory
(is_valid_expression_start, is_valid_expression, is_valid_term):
Return Boolean rather than integer literals.
(get_vunits, get_hunits, get_number_rigidly, get_number, get_integer):
Update call sites of renamed functions. Replace Boolean-valued
integer literals used as Booleans with Boolean literals and annotate
their purposes. (See <https://stackoverflow.com/questions/38076786/\
why-doesnt-c-support-named-parameters>.)
(get_vunits, get_hunits, get_number, get_integer): Replace `0` in
assertions with a communicative predicate. In all of these it's an
unhandled `switch()` case.
Also put space around binary operators in expressions, parenthesize
complex expressions more, tighten some comments, and wrap long lines.
---
ChangeLog | 27 ++++++++
src/roff/troff/number.cpp | 173 ++++++++++++++++++++++++----------------------
2 files changed, 118 insertions(+), 82 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index ba93821b6..8a9579282 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,30 @@
+2022-10-04 G. Branden Robinson <g.branden.robinson@gmail.com>
+
+ * src/roff/troff/number.cpp: Refactor and fix code style nits.
+ Boolify and rename static (local) functions.
+ - parse_expr -> is_valid_expression
+ - start_number -> is_valid_expression_start
+ - parse_term -> is_valid_term
+ Rename preprocessor macro `SCALE_INDICATOR_CHARS` to
+ `SCALING_UNITS`.
+ (is_valid_expression, is_valid_term): Rename parameters and
+ demote them from `int` to `bool`.
+ - scaling_indicator -> scaling_unit (no demotion)
+ - parenthesised -> is_parenthesized
+ - rigid -> is_mandatory
+ (is_valid_expression_start, is_valid_expression, is_valid_term):
+ Return Boolean
+ rather than integer literals.
+ (get_vunits, get_hunits, get_number_rigidly, get_number)
+ (get_integer): Update call sites of renamed functions. Replace
+ Boolean-valued integer literals used as Booleans with Boolean
+ literals and annotate their purposes. (See <https://\
+ stackoverflow.com/questions/38076786/\
+ why-doesnt-c-support-named-parameters>.)
+ (get_vunits, get_hunits, get_number, get_integer): Replace `0`
+ in assertions with a communicative predicate. In all of these
+ it's an unhandled `switch()` case.
+
2022-10-04 G. Branden Robinson <g.branden.robinson@gmail.com>
* src/roff/troff/input.cpp (do_expr_test, do_zero_width): Use
diff --git a/src/roff/troff/number.cpp b/src/roff/troff/number.cpp
index b7ddb918d..348b55726 100644
--- a/src/roff/troff/number.cpp
+++ b/src/roff/troff/number.cpp
@@ -33,16 +33,17 @@ int vresolution = 1;
int units_per_inch;
int sizescale;
-static int parse_expr(units *v, int scaling_indicator,
- int parenthesised, int rigid = 0);
-static int start_number();
+static bool is_valid_expression(units *v, int scaling_unit,
+ bool is_parenthesized,
+ bool is_mandatory = false);
+static bool is_valid_expression_start();
int get_vunits(vunits *res, unsigned char si)
{
- if (!start_number())
+ if (!is_valid_expression_start())
return 0;
units x;
- if (parse_expr(&x, si, 0)) {
+ if (is_valid_expression(&x, si, false /* is_parenthesized */)) {
*res = vunits(x);
return 1;
}
@@ -52,10 +53,10 @@ int get_vunits(vunits *res, unsigned char si)
int get_hunits(hunits *res, unsigned char si)
{
- if (!start_number())
+ if (!is_valid_expression_start())
return 0;
units x;
- if (parse_expr(&x, si, 0)) {
+ if (is_valid_expression(&x, si, false /* is_parenthesized */)) {
*res = hunits(x);
return 1;
}
@@ -67,10 +68,11 @@ int get_hunits(hunits *res, unsigned char si)
int get_number_rigidly(units *res, unsigned char si)
{
- if (!start_number())
+ if (!is_valid_expression_start())
return 0;
units x;
- if (parse_expr(&x, si, 0, 1)) {
+ if (is_valid_expression(&x, si, false /* is_parenthesized */,
+ true /* is_mandatory */)) {
*res = x;
return 1;
}
@@ -80,10 +82,10 @@ int get_number_rigidly(units *res, unsigned char si)
int get_number(units *res, unsigned char si)
{
- if (!start_number())
+ if (!is_valid_expression_start())
return 0;
units x;
- if (parse_expr(&x, si, 0)) {
+ if (is_valid_expression(&x, si, false /* is_parenthesized */)) {
*res = x;
return 1;
}
@@ -93,10 +95,10 @@ int get_number(units *res, unsigned char si)
int get_integer(int *res)
{
- if (!start_number())
+ if (!is_valid_expression_start())
return 0;
units x;
- if (parse_expr(&x, 0, 0)) {
+ if (is_valid_expression(&x, 0, false /* is_parenthesized */)) {
*res = x;
return 1;
}
@@ -124,7 +126,7 @@ int get_vunits(vunits *res, unsigned char si, vunits
prev_value)
*res = prev_value - v;
break;
default:
- assert(0);
+ assert(0 == "unhandled switch case returned by get_incr_number()");
}
return 1;
}
@@ -145,7 +147,7 @@ int get_hunits(hunits *res, unsigned char si, hunits
prev_value)
*res = prev_value - v;
break;
default:
- assert(0);
+ assert(0 == "unhandled switch case returned by get_incr_number()");
}
return 1;
}
@@ -166,7 +168,7 @@ int get_number(units *res, unsigned char si, units
prev_value)
*res = prev_value - v;
break;
default:
- assert(0);
+ assert(0 == "unhandled switch case returned by get_incr_number()");
}
return 1;
}
@@ -187,7 +189,7 @@ int get_integer(int *res, int prev_value)
*res = prev_value - int(v);
break;
default:
- assert(0);
+ assert(0 == "unhandled switch case returned by get_incr_number()");
}
return 1;
}
@@ -195,7 +197,7 @@ int get_integer(int *res, int prev_value)
static incr_number_result get_incr_number(units *res, unsigned char si)
{
- if (!start_number())
+ if (!is_valid_expression_start())
return BAD;
incr_number_result result = ABSOLUTE;
if (tok.ch() == '+') {
@@ -206,46 +208,48 @@ static incr_number_result get_incr_number(units *res,
unsigned char si)
tok.next();
result = DECREMENT;
}
- if (parse_expr(res, si, 0))
+ if (is_valid_expression(res, si, false /* is_parenthesized */))
return result;
else
return BAD;
}
-static int start_number()
+static bool is_valid_expression_start()
{
while (tok.is_space())
tok.next();
if (tok.is_newline()) {
warning(WARN_MISSING, "numeric expression missing");
- return 0;
+ return false;
}
if (tok.is_tab()) {
warning(WARN_TAB, "expected numeric expression, got %1",
tok.description());
- return 0;
+ return false;
}
if (tok.is_right_brace()) {
warning(WARN_RIGHT_BRACE, "expected numeric expression, got right"
"brace escape sequence");
- return 0;
+ return false;
}
- return 1;
+ return true;
}
enum { OP_LEQ = 'L', OP_GEQ = 'G', OP_MAX = 'X', OP_MIN = 'N' };
-#define SCALE_INDICATOR_CHARS "icfPmnpuvMsz"
+#define SCALING_UNITS "icfPmnpuvMsz"
-static int parse_term(units *v, int scaling_indicator,
- int parenthesised, int rigid);
+static bool is_valid_term(units *v, int scaling_unit,
+ bool is_parenthesized, bool is_mandatory);
-static int parse_expr(units *v, int scaling_indicator,
- int parenthesised, int rigid)
+static bool is_valid_expression(units *v, int scaling_unit,
+ bool is_parenthesized,
+ bool is_mandatory)
{
- int result = parse_term(v, scaling_indicator, parenthesised, rigid);
+ int result = is_valid_term(v, scaling_unit, is_parenthesized,
+ is_mandatory);
while (result) {
- if (parenthesised)
+ if (is_parenthesized)
tok.skip();
int op = tok.ch();
switch (op) {
@@ -289,8 +293,9 @@ static int parse_expr(units *v, int scaling_indicator,
return result;
}
units v2;
- if (!parse_term(&v2, scaling_indicator, parenthesised, rigid))
- return 0;
+ if (!is_valid_term(&v2, scaling_unit, is_parenthesized,
+ is_mandatory))
+ return false;
int overflow = 0;
switch (op) {
case '<':
@@ -333,7 +338,7 @@ static int parse_expr(units *v, int scaling_indicator,
}
if (overflow) {
error("addition overflow");
- return 0;
+ return false;
}
*v += v2;
break;
@@ -348,7 +353,7 @@ static int parse_expr(units *v, int scaling_indicator,
}
if (overflow) {
error("subtraction overflow");
- return 0;
+ return false;
}
*v -= v2;
break;
@@ -371,37 +376,37 @@ static int parse_expr(units *v, int scaling_indicator,
}
if (overflow) {
error("multiplication overflow");
- return 0;
+ return false;
}
*v *= v2;
break;
case '/':
if (v2 == 0) {
error("division by zero");
- return 0;
+ return false;
}
*v /= v2;
break;
case '%':
if (v2 == 0) {
error("modulus by zero");
- return 0;
+ return false;
}
*v %= v2;
break;
default:
- assert(0);
+ assert(0 == "unhandled switch case while processing operator");
}
}
return result;
}
-static int parse_term(units *v, int scaling_indicator,
- int parenthesised, int rigid)
+static bool is_valid_term(units *v, int scaling_unit,
+ bool is_parenthesized, bool is_mandatory)
{
int negative = 0;
for (;;)
- if (parenthesised && tok.is_space())
+ if (is_parenthesized && tok.is_space())
tok.next();
else if (tok.ch() == '+')
tok.next();
@@ -417,66 +422,67 @@ static int parse_term(units *v, int scaling_indicator,
// | is not restricted to the outermost level
// tbl uses this
tok.next();
- if (!parse_term(v, scaling_indicator, parenthesised, rigid))
- return 0;
+ if (!is_valid_term(v, scaling_unit, is_parenthesized, is_mandatory))
+ return false;
int tem;
- tem = (scaling_indicator == 'v'
+ tem = (scaling_unit == 'v'
? curdiv->get_vertical_position().to_units()
: curenv->get_input_line_position().to_units());
if (tem >= 0) {
if (*v < INT_MIN + tem) {
error("numeric overflow");
- return 0;
+ return false;
}
}
else {
if (*v > INT_MAX + tem) {
error("numeric overflow");
- return 0;
+ return false;
}
}
*v -= tem;
if (negative) {
if (*v == INT_MIN) {
error("numeric overflow");
- return 0;
+ return false;
}
*v = -*v;
}
- return 1;
+ return true;
case '(':
tok.next();
c = tok.ch();
if (c == ')') {
- if (rigid)
- return 0;
+ if (is_mandatory)
+ return false;
warning(WARN_SYNTAX, "empty parentheses");
tok.next();
*v = 0;
- return 1;
+ return true;
}
- else if (c != 0 && strchr(SCALE_INDICATOR_CHARS, c) != 0) {
+ else if (c != 0 && strchr(SCALING_UNITS, c) != 0) {
tok.next();
if (tok.ch() == ';') {
tok.next();
- scaling_indicator = c;
+ scaling_unit = c;
}
else {
error("expected ';' after scaling unit, got %1",
tok.description());
- return 0;
+ return false;
}
}
else if (c == ';') {
- scaling_indicator = 0;
+ scaling_unit = 0;
tok.next();
}
- if (!parse_expr(v, scaling_indicator, 1, rigid))
- return 0;
+ if (!is_valid_expression(v, scaling_unit,
+ true /* is_parenthesized */, is_mandatory))
+ return false;
tok.skip();
if (tok.ch() != ')') {
- if (rigid)
- return 0;
+ if (is_mandatory)
+ return false;
warning(WARN_SYNTAX, "expected ')', got %1", tok.description());
}
else
@@ -484,11 +490,11 @@ static int parse_term(units *v, int scaling_indicator,
if (negative) {
if (*v == INT_MIN) {
error("numeric overflow");
- return 0;
+ return false;
}
*v = -*v;
}
- return 1;
+ return true;
case '.':
*v = 0;
break;
@@ -506,12 +512,12 @@ static int parse_term(units *v, int scaling_indicator,
do {
if (*v > INT_MAX/10) {
error("numeric overflow");
- return 0;
+ return false;
}
*v *= 10;
if (*v > INT_MAX - (int(c) - '0')) {
error("numeric overflow");
- return 0;
+ return false;
}
*v += c - '0';
tok.next();
@@ -528,11 +534,11 @@ static int parse_term(units *v, int scaling_indicator,
case '=':
warning(WARN_SYNTAX, "empty left operand to '%1' operator", c);
*v = 0;
- return rigid ? 0 : 1;
+ return is_mandatory ? false : true;
default:
warning(WARN_NUMBER, "expected numeric expression, got %1",
tok.description());
- return 0;
+ return false;
}
int divisor = 1;
if (tok.ch() == '.') {
@@ -550,10 +556,10 @@ static int parse_term(units *v, int scaling_indicator,
tok.next();
}
}
- int si = scaling_indicator;
+ int si = scaling_unit;
int do_next = 0;
- if ((c = tok.ch()) != 0 && strchr(SCALE_INDICATOR_CHARS, c) != 0) {
- switch (scaling_indicator) {
+ if ((c = tok.ch()) != 0 && strchr(SCALING_UNITS, c) != 0) {
+ switch (scaling_unit) {
case 0:
warning(WARN_SCALE, "scaling unit invalid in context");
break;
@@ -605,20 +611,23 @@ static int parse_term(units *v, int scaling_indicator,
{
// Convert to hunits so that with -Tascii 'm' behaves as in nroff.
hunits em = curenv->get_size();
- *v = scale(*v, em.is_zero() ? hresolution : em.to_units(), divisor);
+ *v = scale(*v, em.is_zero() ? hresolution : em.to_units(),
+ divisor);
}
break;
case 'M':
{
hunits em = curenv->get_size();
- *v = scale(*v, em.is_zero() ? hresolution : em.to_units(), divisor*100);
+ *v = scale(*v, em.is_zero() ? hresolution : em.to_units(),
+ (divisor * 100));
}
break;
case 'n':
{
// Convert to hunits so that with -Tascii 'n' behaves as in nroff.
- hunits en = curenv->get_size()/2;
- *v = scale(*v, en.is_zero() ? hresolution : en.to_units(), divisor);
+ hunits en = curenv->get_size() / 2;
+ *v = scale(*v, en.is_zero() ? hresolution : en.to_units(),
+ divisor);
}
break;
case 'v':
@@ -635,18 +644,18 @@ static int parse_term(units *v, int scaling_indicator,
*v = scale(*v, sizescale, divisor);
break;
default:
- assert(0);
+ assert(0 == "unhandled switch case when processing scaling unit");
}
if (do_next)
tok.next();
if (negative) {
if (*v == INT_MIN) {
error("numeric overflow");
- return 0;
+ return false;
}
*v = -*v;
}
- return 1;
+ return true;
}
units scale(units n, units x, units y)
@@ -676,24 +685,24 @@ units scale(units n, units x, units y)
vunits::vunits(units x)
{
- // don't depend on the rounding direction for division of negative integers
+ // Don't depend on rounding direction when dividing negative integers.
if (vresolution == 1)
n = x;
else
n = (x < 0
- ? -((-x + vresolution/2 - 1)/vresolution)
- : (x + vresolution/2 - 1)/vresolution);
+ ? -((-x + (vresolution / 2) - 1) / vresolution)
+ : (x + (vresolution / 2) - 1) / vresolution);
}
hunits::hunits(units x)
{
- // don't depend on the rounding direction for division of negative integers
+ // Don't depend on rounding direction when dividing negative integers.
if (hresolution == 1)
n = x;
else
n = (x < 0
- ? -((-x + hresolution/2 - 1)/hresolution)
- : (x + hresolution/2 - 1)/hresolution);
+ ? -((-x + (hresolution / 2) - 1) / hresolution)
+ : (x + (hresolution / 2) - 1) / hresolution);
}
// Local Variables:
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- [groff] 09/32: [troff]: Refactor and fix code style nits.,
G. Branden Robinson <=