[Bug 497756] Review Request: lpg - LALR Parser Generator

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=497756





--- Comment #23 from Mat Booth <fedora at matbooth.co.uk>  2009-07-13 15:51:45 EDT ---
Thanks for the review.

(In reply to comment #22)
> Thanks for untangling this mess, Mat!  The package looks great.  Hopefully we
> can port over dependencies to 2.x APIs and get rid of the compat stuff soon. 
> As for review points, everything looks good to me:
> 
> ? should %{?_smp_mflags} be passed to make?

When I add the smp flags, it deletes the object files before it links the
executable. I'm not really versed well enough with Makefiles to know why and
it's not a massive build so I wasn't convinced it was worth the effort to
figure it out. (Any suggestions welcome, though.) See this build log snippet:

  rm -f core ,* *~ *.bak
  rm -f *.o gmon.out mon.out TAGS tags
  In file included from util.h:4,
                   from option.h:4,
                   from control.h:4,
                   from scanner.cpp:2:
  tuple.h: In member function 'T Stack<T>::Pop() [with T = char*]':
  tuple.h:273: warning: assuming signed overflow does not occur when assuming
that (X - c) > X is always false
  tuple.h: In member function 'char* Scanner::ScanOptions()':
  tuple.h:273: warning: assuming signed overflow does not occur when assuming
that (X - c) > X is always false
  tuple.h:273: warning: assuming signed overflow does not occur when assuming
that (X - c) > X is always false
  g++ CAction.o CTable.o CppAction.o CppTable.o JavaAction.o JavaTable.o
LexStream.o MlAction.o MlTable.o PlxAction.o PlxTable.o PlxasmAction.o
PlxasmTable.o XmlAction.o XmlTable.o Action.o base.o buffer.o code.o control.o
dfa.o diagnose.o dump.o generator.o grammar.o jikespg.o jikespg_act.o
jikespg_init.o jikespg_prs.o option.o parser.o pda.o produce.o resolve.o
scanner.o sp.o symbol.o tab.o table.o util.o -o lpg-linux_x86
  g++: CAction.o: No such file or directory
  g++: CTable.o: No such file or directory
  etc...
  etc...

(In reply to comment #22)
> My only concern is the large number of compiler warnings in the C++ generator
> stuff.  Do you have any thoughts on these?

About 99% of these are an ignored return value from fwrite in buffer.h that
just about every other source file includes, so it looks to me like it's just
the same warning over and over. (I guess it just doesn't care about how many
bytes it has written.)

(In reply to comment #22)
> I apologize for the incredible delay on my review.  This package is good to go.  

No probs. If you're happy enough with my comments above I'll get it booked into
CVS.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.