Skip to content

[Rcpp-devel] bug hunting

13 messages · Dirk Eddelbuettel, Romain Francois, Douglas Bates +1 more

Original
Romain Francois
# 
Hello,

I think the bug reported by Doug in the "Extract a function from a list 
and create a call" thread is now fixed.

The problem was Vector did not have copy constructor and assignement 
operators, so the compiler did generate silly ones for us. Do the vector 
cache was not updated, so the string_proxy was holding an innapropriate 
VECTOR*.

Please let me know if this works now.

In this debuggin battle, I defined the macro RCPP_DEBUG which is similar 
to logTxt but allows printf formatting on the text and also the DEMANGLE 
macro that demangles a c++ class name.


Romain
Dirk Eddelbuettel
#
On 23 March 2010 at 11:14, Romain Francois wrote:
| I think the bug reported by Doug in the "Extract a function from a list 
| and create a call" thread is now fixed.
| 
| The problem was Vector did not have copy constructor and assignement 
| operators, so the compiler did generate silly ones for us. Do the vector 
| cache was not updated, so the string_proxy was holding an innapropriate 
| VECTOR*.

Nice one! Good work.
 
| Please let me know if this works now.
| 
| In this debuggin battle, I defined the macro RCPP_DEBUG which is similar 
| to logTxt but allows printf formatting on the text and also the DEMANGLE 
| macro that demangles a c++ class name.

Very useful, and hadn't come across it. Where did you find that?

Dirk
Romain Francois
# 
Le 23/03/10 12:34, Dirk Eddelbuettel a ?crit :
I have not yet added regression tests for it. volunteers ?
DEMANGLE I've juste factored out of the exceptions.cpp, for the printf 
trick, I've used http://en.wikipedia.org/wiki/C_preprocessor
Dirk Eddelbuettel
# 
FYI, following Doug's lead re '-pedantic', I made that permanent for me via

   edd at ron:~$ grep pedantic .R/*
   .R/Makevars:CFLAGS=-g -O3 -Wall -pipe -pedantic
   .R/Makevars:CXXFLAGS=-g -O3 -Wall -pipe -pedantic

which now reveals a lot of 

   ./RcppCommon.h:205:28: warning: anonymous variadic macros were introduced in C99

as well as 

   from Promise.cpp:22:                                                                     
   ./Rcpp/Vector.h:434:30: warning: ISO C99 requires rest arguments to be used                               
   ./Rcpp/Vector.h:441:29: warning: ISO C99 requires rest arguments to be used                               
   ./Rcpp/Vector.h:477:26: warning: ISO C99 requires rest arguments to be used                               
   ./Rcpp/Vector.h:482:30: warning: ISO C99 requires rest arguments to be used                               
   ./Rcpp/Vector.h:496:31: warning: ISO C99 requires rest arguments to be used   

Not toxic, but something we may want to look at. Without -pedantic,
everything is clean as a whistle.

But most importantly Doug's example now works. 

So release 0.7.11 by the end of the week?

Dirk
Romain Francois
# 
Le 23/03/10 12:49, Dirk Eddelbuettel a ?crit :
We can kill this one with the -Wno-variadic-macros flag
This one, I'm not sure. Any ideas ?

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Dirk Eddelbuettel
    

    

      
      #
    

  


  

      
On 23 March 2010 at 13:16, Romain Francois wrote:

        
| Le 23/03/10 12:49, Dirk Eddelbuettel a ?crit :
| >
| > FYI, following Doug's lead re '-pedantic', I made that permanent for me via
| >
| >     edd at ron:~$ grep pedantic .R/*
| >     .R/Makevars:CFLAGS=-g -O3 -Wall -pipe -pedantic
| >     .R/Makevars:CXXFLAGS=-g -O3 -Wall -pipe -pedantic
| >
| > which now reveals a lot of
| >
| >     ./RcppCommon.h:205:28: warning: anonymous variadic macros were introduced in C99
| 
| We can kill this one with the -Wno-variadic-macros flag

Hmpf. Not great.
 
| > as well as
| >
| >     from Promise.cpp:22:
| >     ./Rcpp/Vector.h:434:30: warning: ISO C99 requires rest arguments to be used
| >     ./Rcpp/Vector.h:441:29: warning: ISO C99 requires rest arguments to be used
| >     ./Rcpp/Vector.h:477:26: warning: ISO C99 requires rest arguments to be used
| >     ./Rcpp/Vector.h:482:30: warning: ISO C99 requires rest arguments to be used
| >     ./Rcpp/Vector.h:496:31: warning: ISO C99 requires rest arguments to be used
| 
| This one, I'm not sure. Any ideas ?

#ifdef protect your DEBUG macro that triggers is?

Dirk

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Dirk Eddelbuettel
    

    

      
      #
    

  


  

      
On 23 March 2010 at 12:48, Romain Francois wrote:

        
| > Nice one! Good work.
| 
| I have not yet added regression tests for it. volunteers ?

rev 939 has one which I just wrote on the short train ride.

Dirk

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Douglas Bates
    

    

      
      #
    

  


  

      
On Tue, Mar 23, 2010 at 8:31 AM, Dirk Eddelbuettel <edd at debian.org> wrote:

            

      
      
      
    

        
Those are the cases where RCPP_DEBUG is being called with a format
string only.  The message seems to indicate that there should be at
least one other argument in the ... list, even if it is not used.  I
got rid of the warnings by appending ", 0" to the format string when
it appears alone.  I would check in the modified file except that
emacs has changed the indentation on all the lines that I changed and
I can't easily recover the original indentation.

            

      
      
      
    

        

      
      
    

  
  


  


      

    

    

      
      

        


  

        

      Dirk Eddelbuettel
    

    

      
      #
    

  


  

      
On 23 March 2010 at 10:04, Douglas Bates wrote:

        
| On Tue, Mar 23, 2010 at 8:31 AM, Dirk Eddelbuettel <edd at debian.org> wrote:

        
| > On 23 March 2010 at 13:16, Romain Francois wrote:

        
| > | Le 23/03/10 12:49, Dirk Eddelbuettel a ?crit :
| > | This one, I'm not sure. Any ideas ?
| >
| > #ifdef protect your DEBUG macro that triggers is?
| 
| Those are the cases where RCPP_DEBUG is being called with a format
| string only.  The message seems to indicate that there should be at
| least one other argument in the ... list, even if it is not used.  I
| got rid of the warnings by appending ", 0" to the format string when
| it appears alone.  I would check in the modified file except that
| emacs has changed the indentation on all the lines that I changed and

Perfect!  I let Emacs re-indent all the time too, and Romain has not yet
beaten me up when I do.  I'd say commit.

| I can't easily recover the original indentation.

That's a feature, not a bug. ;-)

Dirk

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Romain Francois
    

    

      
      #
    

  


  

      
Le 23/03/10 16:04, Douglas Bates a ?crit :

            

      
      
      
    

        
Thanks for this. I commited it earlier.

For future reference, I don't mind if you reindent the code to suit 
emacs, I'm used to Dirk doing it.

Does that mean I should use "soft tabs" (emulated with spaces) ?

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Douglas Bates
    

    

      
      #
    

  


  

      
On Tue, Mar 23, 2010 at 1:29 PM, Romain Francois

        
<romain.francois at dbmail.com> wrote:

            

      
      
      
    

        
I saw that.  Thanks.

            

      
      
      
    

        
I have C++ mode in emacs configured so that certain keystrokes indent
lines and, for me, the indentation step is four.  I believe that
multiple indents are expressed as tabs but I'm not sure.

In other words, my mode hook is
(add-hook 'c++-mode-hook
	  (lambda () (c-set-style "bsd")
	    (setq c-basic-offset 4)))

If Dirk has a preferred style or offset I am happy to modify to
whatever he sends.  He knows the intricacies of emacs better than I
do.

            

      
      
      
    

  
  


  


      

    

    

      
      

        


  

        

      Dirk Eddelbuettel
    

    

      
      #
    

  


  

      
On 23 March 2010 at 19:29, Romain Francois wrote:

        
| For future reference, I don't mind if you reindent the code to suit 
| emacs, I'm used to Dirk doing it.
| 
| Does that mean I should use "soft tabs" (emulated with spaces) ?

I got quite used to just specififying in the header:

   // -*- mode: C++; c-indent-level: 4; c-basic-offset: 4; tab-width: 8 -*-

which means indent by four, offset by four (whatever that does) and use
regular 8 char tabs.  Otherwise I just emacs default values -- and for R I
follow what's in one of the manuals (R Internals?)

        
On 23 March 2010 at 13:36, Douglas Bates wrote:

        
| I have C++ mode in emacs configured so that certain keystrokes indent
| lines and, for me, the indentation step is four.  I believe that
| multiple indents are expressed as tabs but I'm not sure.
| 
| In other words, my mode hook is
| (add-hook 'c++-mode-hook
| 	  (lambda () (c-set-style "bsd")
| 	    (setq c-basic-offset 4)))

That would similar to what I have, albeit with default 'scheme' as "bsd"
Do you know if the per-file ones overrule or not>
 
| If Dirk has a preferred style or offset I am happy to modify to
| whatever he sends.  He knows the intricacies of emacs better than I
| do.

Err, not even close. I understand as much Elisp as I understand Fortran. 

I think our 'non-policy policy' works so far. If we ever feel we need to
correct things, there is alsways GNU indent (and a contributor to RPostgreSQL
actually used it to standardize matters).

I did run 'M-x untabify' on all RInside examples yesterday, though, so that
users get consistent views of the indents so matter what their tabs settings.
But generally the tabs-vs-spaces war is about as useful as the vi-emacs
debate. We emacs users simply know we're right ;-)

Dirk

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Davor Cubranic
    

    

      
      #
    

  


  

      
On 2010-03-23, at 12:03 PM, Dirk Eddelbuettel wrote:

        

            

      
      
      
    

        
The Emacs manual doesn't say it explicitly, but it sounds like file variables override any other settings, e.g., those from mode-hooks. It does say that settings which are really user preferences should not go there, for example. I guess it's open to discussion whether indentation is a user preference or a project policy. :-)

Davor