From b2942d1e729c4cfa8ec9d3bedcdc7ad838a689ef Mon Sep 17 00:00:00 2001 From: ZyX Date: Thu, 13 Apr 2017 19:16:32 +0300 Subject: eval: Change the point at which arg_errmsg and its length are changed Ref #6437 --- src/nvim/eval/typval.c | 22 ++++++++++++++++------ src/nvim/eval/typval.h | 11 +++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) (limited to 'src/nvim/eval') diff --git a/src/nvim/eval/typval.c b/src/nvim/eval/typval.c index eb6db9547b..c29c67124f 100644 --- a/src/nvim/eval/typval.c +++ b/src/nvim/eval/typval.c @@ -2045,11 +2045,14 @@ bool tv_islocked(const typval_T *const tv) /// /// @param[in] lock Lock status. /// @param[in] name Variable name, used in the error message. -/// @param[in] name_len Variable name length. +/// @param[in] name_len Variable name length. Use #TV_TRANSLATE to translate +/// variable name and compute the length. Use #TV_CSTRING +/// to compute the length with strlen() without +/// translating. /// /// @return true if variable is locked, false otherwise. -bool tv_check_lock(const VarLockStatus lock, const char *const name, - const size_t name_len) +bool tv_check_lock(const VarLockStatus lock, const char *name, + size_t name_len) FUNC_ATTR_WARN_UNUSED_RESULT { const char *error_message = NULL; @@ -2068,10 +2071,17 @@ bool tv_check_lock(const VarLockStatus lock, const char *const name, } assert(error_message != NULL); - const char *const unknown_name = _("Unknown"); + if (name == NULL) { + name = _("Unknown"); + name_len = strlen(name); + } else if (name_len == TV_TRANSLATE) { + name = _(name); + name_len = strlen(name); + } else if (name_len == TV_CSTRING) { + name_len = strlen(name); + } - emsgf(_(error_message), (name != NULL ? name_len : strlen(unknown_name)), - (name != NULL ? name : unknown_name)); + emsgf(_(error_message), (int)name_len, name); return true; } diff --git a/src/nvim/eval/typval.h b/src/nvim/eval/typval.h index 7eab22bc12..df46222067 100644 --- a/src/nvim/eval/typval.h +++ b/src/nvim/eval/typval.h @@ -423,6 +423,17 @@ static inline bool tv_is_func(const typval_T tv) return tv.v_type == VAR_FUNC || tv.v_type == VAR_PARTIAL; } +/// Specify that argument needs to be translated +/// +/// Used for size_t length arguments to avoid calling gettext() and strlen() +/// unless needed. +#define TV_TRANSLATE (SIZE_MAX) + +/// Specify that argument is a NUL-terminated C string +/// +/// Used for size_t length arguments to avoid calling strlen() unless needed. +#define TV_CSTRING (SIZE_MAX - 1) + #ifdef INCLUDE_GENERATED_DECLARATIONS # include "eval/typval.h.generated.h" #endif -- cgit From 276ee1f7fb989b931a9ddfabfd4aaf1782bcbb77 Mon Sep 17 00:00:00 2001 From: ZyX Date: Thu, 13 Apr 2017 23:42:51 +0300 Subject: eval: Add comment regarding why special values are needed --- src/nvim/eval/typval.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/nvim/eval') diff --git a/src/nvim/eval/typval.c b/src/nvim/eval/typval.c index c29c67124f..b70554c1ef 100644 --- a/src/nvim/eval/typval.c +++ b/src/nvim/eval/typval.c @@ -2050,6 +2050,12 @@ bool tv_islocked(const typval_T *const tv) /// to compute the length with strlen() without /// translating. /// +/// Both #TV_… values are used for optimization purposes: +/// variable name with its length is needed only in case +/// of error, when no error occurs computing them is +/// a waste of CPU resources. This especially applies to +/// gettext. +/// /// @return true if variable is locked, false otherwise. bool tv_check_lock(const VarLockStatus lock, const char *name, size_t name_len) -- cgit From b54e5c220f0b1bff31ce65c6988f70cbb9780b5e Mon Sep 17 00:00:00 2001 From: ZyX Date: Fri, 14 Apr 2017 00:12:05 +0300 Subject: unittests: Add a test for TV_CSTRING MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not using enum{} because SIZE_MAX exceeds integer and I do not really like how enum definition is described in C99: 1. Even though all values must fit into the chosen type (6.7.2.2, p 4) the type to choose is still implementation-defined. 2. 6.4.4.3 explicitly states that “an identifier declared as an enumeration constant has type `int`”. So it looks like “no matter what type was chosen for enumeration, constants will be integers”. Yet the following simple program: #include #include #include enum { X=SIZE_MAX }; int main(int argc, char **argv) { printf("x:%zu m:%zu t:%zu v:%zu", sizeof(X), sizeof(SIZE_MAX), sizeof(size_t), (size_t)X); } yields one of the following using different compilers: - clang/gcc/pathcc: `x:8 m:8 t:8 v:18446744073709551615` - pcc/tcc: `x:4 m:8 t:8 v:1844674407370955161` If I remove the cast of X to size_t then pcc/tcc both yield `x:4 m:8 t:8 v:4294967295`, other compilers’ output does not change. All compilers were called with `$compiler -std=c99 -xc -` (feeding program from echo), except for `tcc` which has missing `-std=c99`. `pcc` seems to ignore the argument though: it is perfectly fine with `-std=c1000`. --- src/nvim/eval/typval.h | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'src/nvim/eval') diff --git a/src/nvim/eval/typval.h b/src/nvim/eval/typval.h index df46222067..0f659727ee 100644 --- a/src/nvim/eval/typval.h +++ b/src/nvim/eval/typval.h @@ -17,6 +17,7 @@ #include "nvim/pos.h" // for linenr_T #include "nvim/gettext.h" #include "nvim/message.h" +#include "nvim/macros.h" /// Type used for VimL VAR_NUMBER values typedef int varnumber_T; @@ -434,6 +435,12 @@ static inline bool tv_is_func(const typval_T tv) /// Used for size_t length arguments to avoid calling strlen() unless needed. #define TV_CSTRING (SIZE_MAX - 1) +#ifdef UNIT_TESTING +// Do not use enum constants, see commit message. +EXTERN const size_t kTVCstring INIT(= TV_CSTRING); +EXTERN const size_t kTVTranslate INIT(= TV_TRANSLATE); +#endif + #ifdef INCLUDE_GENERATED_DECLARATIONS # include "eval/typval.h.generated.h" #endif -- cgit From 31fd6d4bbf80d0f50893ab6144aa5eb70c95c351 Mon Sep 17 00:00:00 2001 From: ZyX Date: Fri, 14 Apr 2017 23:57:44 +0300 Subject: eval/typval: Do not translate tv_clear argument, this is useless --- src/nvim/eval/typval.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'src/nvim/eval') diff --git a/src/nvim/eval/typval.c b/src/nvim/eval/typval.c index b70554c1ef..d5177138b0 100644 --- a/src/nvim/eval/typval.c +++ b/src/nvim/eval/typval.c @@ -1799,11 +1799,13 @@ static inline void _nothing_conv_dict_end(typval_T *const tv, #define TYPVAL_ENCODE_NAME nothing #define TYPVAL_ENCODE_FIRST_ARG_TYPE const void *const #define TYPVAL_ENCODE_FIRST_ARG_NAME ignored +#define TYPVAL_ENCODE_TRANSLATE_OBJECT_NAME #include "nvim/eval/typval_encode.c.h" #undef TYPVAL_ENCODE_SCOPE #undef TYPVAL_ENCODE_NAME #undef TYPVAL_ENCODE_FIRST_ARG_TYPE #undef TYPVAL_ENCODE_FIRST_ARG_NAME +#undef TYPVAL_ENCODE_TRANSLATE_OBJECT_NAME #undef TYPVAL_ENCODE_ALLOW_SPECIALS #undef TYPVAL_ENCODE_CONV_NIL @@ -1837,12 +1839,14 @@ static inline void _nothing_conv_dict_end(typval_T *const tv, /// @param[in,out] tv Value to free. void tv_clear(typval_T *const tv) { - static char *objname = NULL; // cached because gettext() is slow. #6437 - if (objname == NULL) { - objname = xstrdup(_("tv_clear() argument")); - } if (tv != NULL && tv->v_type != VAR_UNKNOWN) { - const int evn_ret = encode_vim_to_nothing(NULL, tv, objname); + // WARNING: do not translate the string here, gettext is slow and function + // is used *very* often. At the current state encode_vim_to_nothing() does + // not error out and does not use the argument anywhere. + // + // If situation changes and this argument will be used, translate it in the + // place where it is used. + const int evn_ret = encode_vim_to_nothing(NULL, tv, "tv_clear() argument"); (void)evn_ret; assert(evn_ret == OK); } -- cgit From c289986c89dd0189ed8ab709bf2eb822c493542a Mon Sep 17 00:00:00 2001 From: ZyX Date: Sat, 15 Apr 2017 00:08:50 +0300 Subject: eval/encode: Do translate “… argument” strings, but only in conv_error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/nvim/eval/encode.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'src/nvim/eval') diff --git a/src/nvim/eval/encode.c b/src/nvim/eval/encode.c index d74913a481..c058c3a9c0 100644 --- a/src/nvim/eval/encode.c +++ b/src/nvim/eval/encode.c @@ -179,9 +179,9 @@ static int conv_error(const char *const msg, const MPConvStack *const mpstack, } } } - EMSG3(msg, objname, (kv_size(*mpstack) == 0 - ? _("itself") - : (char *) msg_ga.ga_data)); + emsgf(msg, _(objname), (kv_size(*mpstack) == 0 + ? _("itself") + : (char *)msg_ga.ga_data)); ga_clear(&msg_ga); return FAIL; } @@ -790,7 +790,7 @@ char *encode_tv2string(typval_T *tv, size_t *len) garray_T ga; ga_init(&ga, (int)sizeof(char), 80); const int evs_ret = encode_vim_to_string(&ga, tv, - "encode_tv2string() argument"); + N_("encode_tv2string() argument")); (void)evs_ret; assert(evs_ret == OK); did_echo_string_emsg = false; @@ -818,7 +818,7 @@ char *encode_tv2echo(typval_T *tv, size_t *len) ga_concat(&ga, tv->vval.v_string); } } else { - const int eve_ret = encode_vim_to_echo(&ga, tv, ":echo argument"); + const int eve_ret = encode_vim_to_echo(&ga, tv, N_(":echo argument")); (void)eve_ret; assert(eve_ret == OK); } @@ -841,7 +841,8 @@ char *encode_tv2json(typval_T *tv, size_t *len) { garray_T ga; ga_init(&ga, (int)sizeof(char), 80); - const int evj_ret = encode_vim_to_json(&ga, tv, "encode_tv2json() argument"); + const int evj_ret = encode_vim_to_json(&ga, tv, + N_("encode_tv2json() argument")); if (!evj_ret) { ga_clear(&ga); } -- cgit