gnupic: Thread: Re: [gnupic] gpasm: .direct macro doesn't work

On 10/5/07, Scott Dattalo ####@####.#### wrote:
>
>
> ...
>
But I discovered that the .direct macros now fail.

...
>
> The error is from gputils/gputils/gpasm/evaluate.c line 84.
>
> David, I know you have made many changes to gpasm recently. Do you know
> what may be happening here?

The problem doesn't look familiar, but it's quite likely I caused it
indirectly with a different change. I can probably look at it tonight or
this weekend and try to dig something up.

David Barnett

On Thu, 4 Oct 2007 21:50:02 -0700 (PDT)
"Scott Dattalo" ####@####.#### wrote:

> ...
> But I discovered that the .direct macros now fail.
> ...
> 
> David, I know you have made many changes to gpasm recently. Do you
> know what may be happening here?
I think I know exactly what's happening now. Prior to 6/17/07 a
single-character string literal was considered identical to a character
literal. For example "e" meant the same as 'e'. This was a hack for
some fancy coercion in MPLAB and meant that it was impossible to
represent the string literal "e". I made some changes so that this
coercion happened explicitly in the parser or later. The two cases
where I applied the coercion were for 8-bit literal instruction
operands and higher-level expression operands. It looks like directives
like '.direct' were one case that got overlooked. I attached a diff of
the relevant changes so you can see exactly what I'm talking about.

For now you can work around this by changing the "C", "a", and "e"
literals to single-quoted literals ('C', 'a', and 'e'). You could also
apply the diff file attached in reverse and rebuild if you need, but
that's probably even more of a hassle. In terms of a real solution, if
we can figure out exactly which cases still need to be coerced, we can
avoid similar problems. I'm not sure how you feel about the original
fix, but I'd rather not revert permanently since I feel like not being
able to represent single-character string literals is a real flaw.

David Barnett

On Fri, 5 Oct 2007 22:19:36 -0400
David ####@####.#### wrote:

> On Thu, 4 Oct 2007 21:50:02 -0700 (PDT)
> "Scott Dattalo" ####@####.#### wrote:
> 
> > ...
> > But I discovered that the .direct macros now fail.
> > ...
> > 
> > David, I know you have made many changes to gpasm recently. Do you
> > know what may be happening here?
> ...
> I made some changes so that this coercion happened explicitly in the
> parser or later.
> ...
I forgot to mention that you can see the bug report for the original
problem here:
https://sourceforge.net/tracker/?func=detail&atid=431665&aid=1081202&group_id=41924

That might help if you have comments to add to the report or if I didn't
explain it very well.

David Barnett

On Fri, 2007-10-05 at 22:19 -0400, David wrote:
On Thu, 4 Oct 2007 21:50:02 -0700 (PDT)
> "Scott Dattalo" ####@####.#### wrote:
>
> > ...
> > But I discovered that the .direct macros now fail.
> > ...
> >
> > David, I know you have made many changes to gpasm recently. Do you
> > know what may be happening here?
> I think I know exactly what's happening now. Prior to 6/17/07 a
> single-character string literal was considered identical to a character
> literal. For example "e" meant the same as 'e'. This was a hack for
> some fancy coercion in MPLAB and meant that it was impossible to
> represent the string literal "e".

Hi David,

Maybe I don't understand the whole issue, but would it have made more
sense to keep the parser and lexer the same and make the coercion
decisions at a higher level?

For example, based on the hints you gave me here, I made a simple change
to the 'do_direct()' function.

Index: directive.c
===================================================================
RCS file: /cvsroot/gputils/gputils/gpasm/directive.c,v
retrieving revision 1.75
diff -u -r1.75 directive.c
--- directive.c 26 Jun 2007 21:40:54 -0000      1.75
+++ directive.c 6 Oct 2007 04:39:37 -0000
@@ -1263,6 +1263,7 @@
     gperror(GPE_OBJECT_ONLY, NULL);
   } else if (enforce_arity(arity, 2)) {
     p = HEAD(parms);
+    coerce_str1(p);
     value = maybe_evaluate(p);
     if ((value < 0) || (value > 255)) {
       gperror(GPE_RANGE, NULL);


Now gpasm works again (for me). I'll check this into CVS.

BTW, I ran the test suite and found that there are several failures there
(I verified this change add no new ones). Should we be concerned about
these.

Scott

On Fri, 5 Oct 2007 21:44:06 -0700 (PDT)
"Scott Dattalo" ####@####.#### wrote:

> On Fri, 2007-10-05 at 22:19 -0400, David wrote:
> > I think I know exactly what's happening now. Prior to 6/17/07 a
> > single-character string literal was considered identical to a
> > character literal. For example "e" meant the same as 'e'. This was
> > a hack for some fancy coercion in MPLAB and meant that it was
> > impossible to represent the string literal "e".
> 
> Hi David,
> 
> Maybe I don't understand the whole issue, but would it have made more
> sense to keep the parser and lexer the same and make the coercion
> decisions at a higher level?
The changes to the lexer and some of the changes to the parser are
absolutely necessary to make any coercion decisions possible at higher
levels. In the past the lexer made the decision at the beginning for all
levels. You can see now that decision is deferred to 2 places at higher
levels.

> For example, based on the hints you gave me here, I made a simple
> change to the 'do_direct()' function.
That is similar to what I had in mind, and it certainly won't cause
any problems for the time being. I totally understand your rush to
patch it over and get back to gpsim issues. My concern was that if
possible I don't want to do this case-by-case and bit-by-bit, and I'm
fairly certain the '.direct' directive isn't the only place we'll see
this issue. I'm hoping to find one or a few places to put most of the
coercions for "any directive expecting a character literal but not
string literal". So far we have for that category: 8-bit literal
instruction operands + expression operands + the first operand
of .direct, and you can see how the 3rd category sounds a lot more
specific. Could be it's not much of a problem, and it could be there's
no other option, but I'm hoping not to sprinkle these changes through
the sources.

I know you're not too much on top of the intricacies of gputils, but I
thought you or someone else might have some ideas or concerns I'm not
thinking of.

> BTW, I ran the test suite and found that there are several failures
> there (I verified this change add no new ones). Should we be
> concerned about these.
You're talking about the gpsim test suite, right? I had only been
keeping an eye on gputils regression tests, but if I'm making your
tests fail it means you've got expectations of gputils that I'm not
fulfilling, and we should definitely take a hard look at that.

David Barnett

On Sat, 2007-10-06 at 13:51 -0400, David wrote:

> The changes to the lexer and some of the changes to the parser are
> absolutely necessary to make any coercion decisions possible at higher
> levels. In the past the lexer made the decision at the beginning for all
> levels. You can see now that decision is deferred to 2 places at higher
> levels.

I'll just have to take your word for this... I know the MPASM language is
quite fickle.


> I know you're not too much on top of the intricacies of gputils, but I
> thought you or someone else might have some ideas or concerns I'm not
> thinking of.

I've gained some experience with flex and bison over the years (read that
as I've been burned many times) and know there are many ways to implement
a language. In my opinion, the character literal and strings are
fundamental types and should be kept separate up to the point where the
language is unambiguous and can perform an implicit type cast. I don't
think the MPASM language is rigorous enough to say that *all* single
character strings can be treated as literal characters.

Consider this situation: suppose there is a particular construct that
requires a string but not a literal character. If the user expresses a
single character string, then the parser will unconditionally convert it
to a literal. And that's an error. This is to be compared against the
other situation where the language can accept either a literal or a single
character string. The implicit type cast causes no problem in this case.
For the first case, the grammar of the language implies that there should
be a separate parser rule for the two different operand types.


>> BTW, I ran the test suite and found that there are several failures

> You're talking about the gpsim test suite, right?

Actually no, I'm talking about gpasm. When I run through the test suite I
see:

-----------------------------------------------------------
./gpasm.mchip testing complete
-----------------------------------------------------------

423 files tested without intentional errors
399 compiled without errors
397 generated identical hex files

74 files tested with intentional errors
73 generated errors during compilation

Scott

On Mon, 8 Oct 2007 07:22:39 -0700 (PDT)
"Scott Dattalo" ####@####.#### wrote:

> On Sat, 2007-10-06 at 13:51 -0400, David wrote:
> > I know you're not too much on top of the intricacies of gputils,
> > but I thought you or someone else might have some ideas or concerns
> > I'm not thinking of.
> 
> In my opinion, the character literal and strings are fundamental
> types and should be kept separate up to the point where the language
> is unambiguous and can perform an implicit type cast. I don't think
> the MPASM language is rigorous enough to say that *all* single
> character strings can be treated as literal characters.
I definitely agree. I'm not sure if you missed this point, but the
original bug report was that DA "x" put 1 byte ('x') instead of 2 ('x',
'\0') into memory, because it coerced to a character when a string
would have worked. I believe MPLAB behaves as expected (surprised?) on
that point. So there are times when the coercion *needs* to be late, and
fixing that led to your bug. Now instead of "too much" of the feature
changing correct behavior, there's "too little" of it so that things
like .direct "e" weren't being coerced.

FYI, I think the "evaluate" function in gpasm/evaluate.c gives me a
"choke point" to coerce most (all?) string literals that must have a
character value. It's supposed to return an integer type for a given
expression, so the caller should always expect a character and never a
string (strings don't have integer values). There's a bit of a
side-effects vs. redundancy issue with the "can_evaluate" function, but
it seems like the way to go, anyway. I'll give it a little more thought
before I change it in CVS.

> >> BTW, I ran the test suite and found that there are several failures
> > You're talking about the gpsim test suite, right?
> 
> Actually no, I'm talking about gpasm.
Ah, yes. Those aren't new at all. If you remember from the discussion
way back when, there was a typo in the test scripts that hid some
failures. Those errors have AFAIK always been there, and we're just
slowly working toward full MPASM compatibility. I seem to remember a
lot of those failures are caused by one bug:
http://sourceforge.net/tracker/index.php?func=detail&aid=564641&group_id=41924&atid=431668
Instead of giving warnings like "Warning: directive found in column 1",
gpasm completely ignores things with the wrong indentation and fails
from side-effects.

Anyway, I fixed the typo way back then but also commented out the
failure part until we iron out those errors. I just diff the output
from one test run to the next to check for new errors for now.

David Barnett

David wrote:

with regards to gpasm regression test failures
> Ah, yes. Those aren't new at all. If you remember from the discussion
> way back when, there was a typo in the test scripts that hid some
> failures.

The time constant of my memory is very short (and seems to be getting 
shorter for some reason), but I think I do recall something about this. 
I just looked in there to see that the #v tests are present. Maybe I can 
take a look at adding support for these.

Do you know if we still receive regression tests from Microchip?


Scott