lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[lmi] [PATCH] Fix trailing whitespace detection in stream_cast()


From: Vadim Zeitlin
Subject: [lmi] [PATCH] Fix trailing whitespace detection in stream_cast()
Date: Mon, 27 Jan 2020 18:13:24 +0100

 Hello,

 I'd like to return to the issue first discussed in this message:

        https://lists.nongnu.org/archive/html/lmi/2018-11/msg00034.html

This might look like a minor problem, but it's really annoying to have to
juggle with the additional patch when testing lmi either with clang or
MSVC, so I'd like to propose the patch at
 
        https://github.com/vadz/lmi/pull/136

for inclusion in lmi master.

 This patch simply exchanges the order of ws() and eof() checks, which is
sufficient for working around libstdc++ bug which results in the currently
clearly wrong code (which checks that there is _no_ whitespace left in the
string instead of testing that some whitespace is left) still working when
using this standard library implementation.

 The advantages of this patch are:

- It makes things work (i.e. the new unit tests pass, but, arguably even
  more importantly, lmi can now construct Input objects successfully) when
  using clang/MSVC.

- It fixes the "!(interpreter >> std::ws)" check which just doesn't make
  any sense when reading the code.

- It improves the error message in case there are actually any trailing
  spaces, as it was clearly supposed to be "Trailing whitespace remains" 
  and not the more generic "Unconverted data remains".

 The only tiny disadvantage I see is the tiny increase in the code
complexity due to using nested ifs instead of a flat chain of them. If this
is really crucial, I would argue for simply removing the specific check for
trailing whitespace, i.e. dropping the entire "else if" branch testing for
it. Clearly, this code never did anything useful in the official lmi builds
using gcc/libstdc++ anyhow, so we wouldn't lose much by removing it and
losing its more specific error message (which, again, was never given to
the users in practice anyhow).

 Thanks in advance for applying this patch,
VZ

Attachment: pgpRv392I_t9h.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]