"Mismatched parentheses" error discrepancy between builds

[tom-seddon]

The following code builds fine with the 1.09 EXE from the repo, but produces a Mismatched parentheses error with an OS X build from the proposed-updates branch.

org 0:Q%=255:equb (128ANDQ%)

Same issue with this:

org 0:Q%=255:equb (128AND Q%)

The following code builds OK on both:

org 0:Q%=255:equb (128 ANDQ%)

I found this trying to build Kieran's buildable copy of Elite - pretty sure this is valid BASIC assembler code, so it should probably be valid BeebAsm code too.

Thanks,

--Tom

[mungre]

I cannot reproduce this with proposed-updates on Windows. The only two changes since the release of 1.09 have been to the handling of line endings, but I don't think this is the problem because one of the lines you tweaked has additional stuff after the failing close bracket.

I'll give it a go on Linux later.

[ZornsLemma]

I've had a go at this on Linux (Ubuntu 16.04.4, x64) and I can't reproduce it either. I checked out 52639cd from Kieran's repo to avoid your fix and it still built fine with the proposed-updates beebasm. I also tried your first example above without any problem. I've tried building with both gcc 5.4.0 and clang 3.8.0.

A few miscellaneous suggestions:

• Can you attach a binary (e.g. gzipped) version of one of the failing examples above, so we can be sure there's no subtle difference in line endings when we try to reproduce it? And similarly give an example command line to invoke beebasm with just in case this (implausibly) turns out to have some effect?
• Can you perhaps add some debug output to the four places in the code where MismatchedParentheses is thrown? If we can see which of the four places it's occurring and perhaps the exact values of m_line and m_column it might help pinpoint the problem.

[tom-seddon]

Thanks for checking it. I've now built and tried Linux and Windows versions of 1.10-pre, and they're unaffected for me, too. Only the OS X build exhibits this problem. So I'll dig into it myself and try to figure it out, or at least narrow things down. More news when I've got it.

--Tom

P.S. the repro steps for me are this, typed in at the shell prompt, on OS X (giving the error) or Linux (assembles, then moans about missing SAVE):

echo 'org 0:Q%=255:equb (128ANDQ%)' > /tmp/test.txt
beebasm -i /tmp/test.txt

test.txt.gz

[tom-seddon]

It looks like libc++ and libstdc++ behave differently when reading a double that's followed by additional chars. Test program here: https://github.com/tom-seddon/beebasm_iostream_thing

As a quick test, I tried replacing the first get a number case in LineParser::GetValue with the following, and that seemed to make it parse correctly. However I tested this no further than that - it was just a quick thing to make sure this is on the right track.

		const char *base=m_line.c_str()+m_column;
		char *ep;
		value=strtod(base,&ep);
		m_column+=ep-base;

Thanks,

--Tom

[mungre]

As far as I can see this is a bug in libc++. From a quick glance at the code I think it's eating characters that could appear in a hex float constant, and then failing, even if the current number is not a hex float.

I posted a message to their mailing list (which is awaiting moderation). If they fix it, great!

[ZornsLemma]

Thanks for investigating that, I can't believe you even created a README.md for the test program - that's going the extra mile!

Having had a quick grep around the beebasm code, I think we only use '>>' to do input in LineParser::GetValue() and SymbolTable::AddCommandLineSymbol(). Should we maybe rewrite all these uses to use C library I/O as in Tom's fragment above, or could that suffer platform differences as well? I'm no expert on C or C++ standard library I/O.

Just seen mungre's comment as I was writing this - thanks for that, let's wait and see what comes back from their mailing list before we consider workarounds then...

[tom-seddon]

I create a README for everything, pretty much, mainly so I can remember WTH it was for when I find it again later. The copy and pasted output might look a bit excessively belt and braces, but it would either have gone there or here :)

This did feel a bit like a bug to me, especially given the different behaviour with ints, and the libc++/libstdc++ discrepancy, but I'm not familiar with iostreams myself either, so I wouldn't like to say which case is going wrong here. I went for strtod because it (like strtol, strtoull, etc.) does promise to just parse as much as it can, and ignore the rest - the ep parameter is there so the caller can determine how much was consumed. And it looks like the syntax might be standardized: https://en.cppreference.com/w/c/string/byte/strtof

(Well, the behaviour of iostreams is probably standardized too, so whether that promise would be worth anything in practice...)

--Tom