Re: Issue 5415: Fix type-conversion warnings in parser and lexer (issue 348990043 by address@hidden)

[nine . fierce . ballads]

Reviewers: dak,


https://codereview.appspot.com/348990043/diff/1/lily/include/keyword.hh
File lily/include/keyword.hh (right):

https://codereview.appspot.com/348990043/diff/1/lily/include/keyword.hh#newcode40
lily/include/keyword.hh:40: int lookup (char const *s) const;
On 2018/09/09 09:45:53, dak wrote:
> This seems like a change not in line with what we do elsewhere.  What
was the
> problem mandating this change?

The compiler complained that Lily_lexer::lookup_keyword () was calling
this method and converting the vsize to an int for return.

lookup () gets its result from Keyword_ent::tokcode_, which is an int,
so this small change makes it int all the way through.  I see this as
the path of least resistance in a class that's already commented
"junkme, use hash table" (line 33).

https://codereview.appspot.com/348990043/diff/1/lily/parser.yy
File lily/parser.yy (right):

https://codereview.appspot.com/348990043/diff/1/lily/parser.yy#newcode161
lily/parser.yy:161: if (Token != END_OF_FILE)                           \
On 2018/09/09 09:45:53, dak wrote:
> yet in use?  So instead of rewording in a dubious manner, the whole
if-condition
> should be thrown out (I don't remember what it was good for) and the
indentation

Will do. Thanks.

Description:
https://sourceforge.net/p/testlilyissues/issues/5415/

two commits:

* lexer: move repeated code into a skip_chars() method.  In passing,
  fix type-conversion warnings produced by g++ 8.0.1 on x86_64.

* fix some parsing type-conversion warnings observed with g++ 8.0.1 on
  x86_64


Please review this at https://codereview.appspot.com/348990043/

Affected files (+20, -41 lines):
  M lily/includable-lexer.cc
  M lily/include/includable-lexer.hh
  M lily/include/input.hh
  M lily/include/keyword.hh
  M lily/input.cc
  M lily/keyword.cc
  M lily/lexer.ll
  M lily/parser.yy


Index: lily/includable-lexer.cc
diff --git a/lily/includable-lexer.cc b/lily/includable-lexer.cc
index  
8567ec91718c470e0e24741902d85911dff391b3..c1761b779159fd2c97223617e3b49c353834cb99  
100644
--- a/lily/includable-lexer.cc
+++ b/lily/includable-lexer.cc
@@ -157,3 +157,10 @@ Includable_lexer::get_source_file () const
     return 0;
   return include_stack_.back ();
 }
+
+void Includable_lexer::skip_chars (size_t count)
+{
+  for (size_t i = 0; i < count; ++i)
+    yyinput ();
+  char_count_stack_.back () += count;
+}
Index: lily/include/includable-lexer.hh
diff --git a/lily/include/includable-lexer.hh  
b/lily/include/includable-lexer.hh
index  
bfc3d81718904244e7b4638735f9105bb905fdfd..1f2e6d9f3b37d8d38723d81762bc28b5e39bd22a  
100644
--- a/lily/include/includable-lexer.hh
+++ b/lily/include/includable-lexer.hh
@@ -41,7 +41,7 @@ class Includable_lexer : public yyFlexLexer
 protected:
   bool close_input ();
   vector<Source_file *> include_stack_;
-  vector<int> char_count_stack_;
+  vector<size_t> char_count_stack_;

 public:

@@ -58,6 +58,8 @@ public:
   void new_input (const string &name, string data, Sources *);

   char const *here_str0 () const;
+
+  void skip_chars (size_t count);
 };

 #endif // INCLUDABLE_LEXER_HH
Index: lily/include/input.hh
diff --git a/lily/include/input.hh b/lily/include/input.hh
index  
cfe1b69ec627ebe7752122bc65ab5860f60f4f95..5024f29e5c739a4ede71b2427a6e33ee2877e843  
100644
--- a/lily/include/input.hh
+++ b/lily/include/input.hh
@@ -37,8 +37,9 @@ public:
   static SCM equal_p (SCM, SCM);
   SCM mark_smob () const;
   Source_file *get_source_file () const;
-  char const *start () const;
-  char const *end () const;
+  char const *start () const { return start_; }
+  char const *end () const { return end_; }
+  size_t size () const { return end_ - start_; }

   void set (Source_file *, char const *, char const *);
   void error (const string&) const;
Index: lily/include/keyword.hh
diff --git a/lily/include/keyword.hh b/lily/include/keyword.hh
index  
0f36fe812b2229bdbf951fdcf3844012b805ea68..8eb3b0f3c2e200943097c85988e30648976118c5  
100644
--- a/lily/include/keyword.hh
+++ b/lily/include/keyword.hh
@@ -37,7 +37,7 @@ struct Keyword_table
   vector<Keyword_ent> table_;

   Keyword_table (Keyword_ent *);
-  vsize lookup (char const *s) const;
+  int lookup (char const *s) const;
 };

 #endif // KEYWORD_HH
Index: lily/input.cc
diff --git a/lily/input.cc b/lily/input.cc
index  
b6aed9e2d1296b74182d9aca7f2921c6f9a04001..3162f7519212963d8a290f335c96e68ca6dc79df  
100644
--- a/lily/input.cc
+++ b/lily/input.cc
@@ -208,18 +208,6 @@ Input::get_source_file () const
   return source_file_;
 }

-char const *
-Input::start () const
-{
-  return start_;
-}
-
-char const *
-Input::end () const
-{
-  return end_;
-}
-
 static SCM
 with_location_hook_0 (void *it)
 {
Index: lily/keyword.cc
diff --git a/lily/keyword.cc b/lily/keyword.cc
index  
b5f7947ada203e31550ee041a557f37f439b115c..be7ffa55dde5f245ec0afa7bb7e54039cb2fffe1  
100644
--- a/lily/keyword.cc
+++ b/lily/keyword.cc
@@ -22,7 +22,7 @@ Keyword_table::Keyword_table (Keyword_ent *tab)
   vector_sort (table_, tab_less);
 }

-vsize
+int
 Keyword_table::lookup (char const *s) const
 {
   Keyword_ent e;
@@ -30,5 +30,5 @@ Keyword_table::lookup (char const *s) const
   vsize idx = binary_search (table_, e, tab_less);
   if (idx != VPOS)
     return table_[idx].tokcode_;
-  return VPOS;
+  return -1;
 }
Index: lily/lexer.ll
diff --git a/lily/lexer.ll b/lily/lexer.ll
index  
6734b9b20c3cb1b8717581f45a0c47d5eac28db6..421fea2734cc1b62635ee4d9da6608d27171ecbd  
100644
--- a/lily/lexer.ll
+++ b/lily/lexer.ll
@@ -341,13 +341,7 @@ BOM_UTF8   \357\273\277
        hi.step_forward ();
        SCM sval = ly_parse_scm (hi, be_safe_global && is_main_input_, parser_);
        sval = eval_scm (sval, hi);
-       int n = hi.end () - hi.start ();
-
-       for (int i = 0; i < n; i++)
-       {
-               yyinput ();
-       }
-       char_count_stack_.back () += n;
+       skip_chars (hi.size ());

        if (scm_is_string (sval)) {
                new_input (ly_scm2string (sval), sources_);
@@ -398,12 +392,7 @@ BOM_UTF8   \357\273\277
        if (SCM_UNBNDP (sval))
                error_level_ = 1;

-       int n = hi.end () - hi.start ();
-       for (int i = 0; i < n; i++)
-       {
-               yyinput ();
-       }
-       char_count_stack_.back () += n;
+       skip_chars (hi.size ());

        yylval = sval;
        return SCM_TOKEN;
@@ -413,15 +402,7 @@ BOM_UTF8   \357\273\277
        Input hi = here_input();
        hi.step_forward ();
        SCM sval = ly_parse_scm (hi, be_safe_global && is_main_input_, parser_);
-
-       int n = hi.end () - hi.start ();
-
-       for (int i = 0; i < n; i++)
-       {
-               yyinput ();
-       }
-       char_count_stack_.back () += n;
-
+       skip_chars (hi.size ());
        sval = eval_scm (sval, hi, '$');

        if (YYSTATE == markup && ly_is_procedure (sval))
Index: lily/parser.yy
diff --git a/lily/parser.yy b/lily/parser.yy
index  
639fc97beefff0a2f35affe5afe678da3bdfafea..52c1dc2b5a6efcea3a97b736f483d14025412983  
100644
--- a/lily/parser.yy
+++ b/lily/parser.yy
@@ -158,7 +158,7 @@ Lily_parser::parser_error (Input const *i, Lily_parser  
*parser, SCM *, const str
                if (yychar != YYEMPTY)                                  \
                        parser->lexer_->push_extra_token          \
                                (yylloc, yychar, yylval);               \
-               if (Token)                                              \
+               if (Token != END_OF_FILE)                               \
                        parser->lexer_->push_extra_token          \
                                (Location, Token, Value);               \
                parser->lexer_->push_extra_token (Location, BACKUP);      \