[antlr-interest] [C Target] Problem with ANTLR 3.1b1 - C code generation

[Jim Idle]

On Mon, 2008-06-16 at 11:24 -0400, Garry Iglesias wrote:

>   Hi everybody,
>   Sorry to disturb you, I've been successfully using ANTLR v3 with
> JAVA for 3 months now, for one client, and I'm pretty happy with it.
>  
>   Now I tried to generate a C++ parser for another project, and I was
> quite disapointed by the absence of support for C++

.You should use the 3.1 beta which creates C++ compatible C, as posted
in the API documentation, lots of previous posts and so on. Should have
taken you less time to find this out than to write this email.

>  and the very poor support for C...


Well, that will get you a long way. What do you think is so poor abut
it?


>  First I tried the 3.0.1, which was generated 'good' c sources, but I
> was stuck with de C/C++ interface, I needed to have more 'C++' inside
> my rules, and the 'extern "C" {}' was too restrictive. (Needed a lot
> of 'glue code' to interface the C++ classes).

C++ wasn't supported in 3.0.1


> Then I tried to #include my generated parser files in C into some CPP
> file. It worked well except a 'cast' problem (which I localized in the
> C.stg code generation template...) that was preventing to compile
> things like : TFoo tmp = ANTL3_MALLOC(sizeof(TFoo)).
> I hesitated to patch the stg and recompile ANTLR (still 3.0.1 at that
> time).
>  
>   When I remember about a recent beta, which I installed immediatly.
> Nice :)... But... ;)
>   ...But it seems things are worse !!! 


It is a beta, there will be an update and perhaps in fact a release this
week.


> I still haven't noticed any cast problem (yet... :) ).


But it is worse eh?


>  But I found some weird bug, which seems to show the generated code (I
> assume from the current C .stgs) is buggy. Sorry, I'm surprised,
> (maybe it's on my side but I can't understand how...).


You don;t understand anythign about the internal code generation. The
internal code generation doesn't understand anything about the parameter
types you are passing around - when it does not know anything about the
type, it assumes that it needs setting to NULL. I already have this as
an issue to look into to see something can be done for C++ references
Please remember that this is beta and is the C runtime, adapted for use
with C++ in actions. I don't seem to able to recruit anyone to write
regression tests for the C runtime.

>  
>   First here is a extract of my grammar source :
>  
> [...]
> defIndexed
>  : INDEXED sz=bitWidth id=identifier '[' cnt=immediateInteger ']'
> access=quotedString
>  {
>   SStringPtr l_pidStr=$id.s;
> [...]
>  
> [...]
> identifier returns [SStringPtr s]
>  : quoted=IDENTIFIER
>   { 
>    const char* l_pString=(const char*)($quoted.text->chars);
>    $s = newStringFromLimitedString(1+l_pString,strlen(l_pString)-2);
>   }
>  ;
> [...]
>  
>   This looks alright for me, I had almost the same rule in my JAVA
> project and it works well. (I remind you that this was not a problem
> on ANTLR v3.01)...


Java isn't C++. However in this case it is to do with a change made
outside the C that I first implemented then persuaded everyone to back
out of in the other targets because of this kind of problem. It is
ironic that I didn't want that change, and am getting beaten up over the
change because of a bug where this has been accidentally left in there
in this one case. I thought I had removed them all, obviously I have
not. It is beta, and i normally appreciate the help given me to find
things that are undiscovered by myself.

>  
>   Now here is the generated code by ANTLR :
>  
> // The 'buggy' rule code :
> static MyparserParser_defIndexed_return
> defIndexed(pMyparserParser ctx)
> {
>  [...]
>     id=identifier(ctx);
>     [...]
>     //Here is the generated code for the rule action :
>     {
>   SStringPtr l_pidStr=( id != NULL ? id.s : NULL ); //!! THis is NOT a
> pointer !!


Yep - and how do you expect ANTLR to know that then?



> 
> 
>   As you can see, the 'identifier' rule implementation returns its
> result into a 'by value' struct, whereas the 'defIndexed' rule
> implementation try to see if a 'by reference' (some kind of pointer)
> result exists...
>   I would be greatly surprise to be the first one to find this bug, it
> seems so straightforward... Did I do something wrong ?


Yes - you didn't read any of the prior discussing this, automatic
assumptions about types and NULL and the fact that the C rules already
return a struct by value, which contains your return types, hence you
should not do this. Please use the search to find all the information on
this, then you might not be so surprised by everything in the future.
Essentially you are doing the wrong thing, but the code generator is
generic for all targets and cannot know enough about the context of your
parameters to tell you that you are doing the wrong thing. 


>  Did I miss something ? I'll have another note as I 'digged' into the
> C runtime project, the beta version has several 'vsrulesfiles'
> directory (all empty but one... :) ), and some other 'ghost/empties'
> directory. It seems far more 'work in progress' than the 3.0.1.


No, it is just stuff left over in the tar - again this is beta. When
perforce deletes things from the repository it does not delete the local
directories as quite often this is not what is wanted. Hence when
Terence tars up the release, it comes with empty directories. Why does
this make you believe it is a work in progress? Have you read the C API
documentation, or did you just scan through the directories and assume
you would work it out? A legitimate thing to point out is if I have not
included something like where the vsrules files are, in the docs. I may
not have done, I thought I had, but maybe I didn't yet.


>  
>   Also : I'm a 'warning killer maniac' and compile (using Visual C++
> 2005), in warning level 4, warning as error...]


The output code and the C runtime is compiled using gcc 4.2 and Visual
Studio 2008 using the full warning as error flags, so it will pass 2005
warnings, at least when I say it is released. It is very probable that I
have not found all code generation combinations to pass through the C++
compiler cleanly yet. I do appreciate well explained bug reports. In the
docs I just ask for a bit of respect/politeness, but this only works if
people read the docs I guess. Pay me money then you can complain
bitterly to me. 


>  
>   Finally, as a C++ programmer I don't mind you use classes in your
> parser... It can be a C target only. Just if it can compile 'embedded'
> in a C++ file, or C++ compliant, without warnings. 


Well, in previous posts I called for assistance in testing this, though
I don't remember calling for a bunch of abuse and criticism of something
that i have not yet announced full support for and provide for free in
my spare time (which I don't have a lot of at the moment, hence the
short delay in 3.1 release).


> The most important stuff for a 'C++ ANTLR user' (or for me :) ), is to
> be able to use 'C++' code inside my rules actions. I don't mind if
> ANTLR generated code or ANTLR runtime is C++ or not. I need 'my code'
> to be C++ that's it. :)


That is the intent and I believe what people should be aiming for. Some
people like the whole class stuff though, despite the overheads, so I
will be moving more towards this for the next version of ANTLR, just for
C++ people. However, I am going to write up some documentation about why
this isn't necessarily the best move too.

>  
>   Despite my first contact with the community is to report a bug,
> great job to all of you.
>   Thank you !
>  
> (By the way : ANTLR is a GREAAAT tool, as a long time 'parser geek' I
> enjoy it, and it opens more new perspectives to me :) ).


Well, I am just sorry that my poorly supported beta isn't perfect - I
will make sure I run it all by you before I do anything else of course.
>  
> Garry.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.antlr.org/pipermail/antlr-interest/attachments/20080616/8c43c298/attachment.html