Project

General

Profile

[logo] 
 
Home
News
Activity
About/Contact
Major Tools
  Dinotrace
  Verilator
  Verilog-mode
  Verilog-Perl
Other Tools
  IPC::Locker
  Parallel::Forker
  Voneline
General Info
  Papers

Issue #1658

Verilog::Preproc misdocuments def_exists instead of def_params

Added by Topa Tota 12 days ago. Updated 11 days ago.

Status:
Closed
Priority:
Normal
Assignee:
% Done:

0%


Description

In the package Verilog::Preproc, the callback def_exists doesn't get called at all (at least I haven't found any situation where it gets called). I believe when Verilog:Preproc finds `ifdef, `ifndef, and `elsif, it should call def_exists. However, it is calling def_params instead. Below is an example:

# test.pl
use strict;
use warnings;

package MyPreproc;
use Verilog::Preproc;
use parent qw(Verilog::Preproc);

sub new {
    my $class = shift;
    my $self = $class->SUPER::new(@_);
    bless $self, $class;
    return $self;
}

sub def_params {
    my $self = shift;
    print "def_params called!\n";
    return $self->SUPER::def_params(@_);
}

sub def_exists {
    my $self = shift;
    print "def_exists called!\n";
    return $self->SUPER::def_exists(@_);
}

package main;
use Verilog::Getopt;

my $opt = new Verilog::Getopt();

my $preproc = new MyPreproc();

$preproc->open("test.sv");

while (defined (my $line = $preproc->getline())) { }

# test.sv
`define DEBUG 1

`ifdef DEBUG
`endif

The reason I think it should call def_exists is because in file VPreProc.cpp lines 829 and 845 (Module version: 3.470), m_preprocp->defExists is called when the state is ps_DEFNAME_IFDEF, ps_DEFNAME_IFNDEF, or ps_DEFNAME_ELSIF. I also think checking a token is defined should call both def_exists (VPreProc.cpp line 794) and def_params (VPreProc.cpp line 1186). It only calls def_params.

Also, def_params is an undocumented callback in the docs.

History

#1 Updated by Topa Tota 12 days ago

Looking more through the code.

defExists calls defParams (Preproc.xs line 134) which calls def_params (Preproc.xs line 139). I think the fix is either

  • remove def_exists from docs and replace it with def_params

OR ideally

  • defExists should call def_exists (Preproc.xs line 134). Then def_exists should call def_params (in Preproc.pm) or better to have def_exists not call def_params at all (to give end-user more flexibility).

#2 Updated by Wilson Snyder 12 days ago

  • Subject changed from Verilog::Preproc should call def_exists instead of def_params when parsing `idef, `ifndef, `elsif to Verilog::Preproc misdocuments def_exists instead of def_params
  • Status changed from New to Closed
  • Assignee set to Wilson Snyder

Hi, thanks for your report.

I prefer to keep it backward compatible, so I've committed fixing this as a documentation change, removing def_exists and documenting def_params. Note this also matches up as def_params calls the option handler's defparam.

#3 Updated by Topa Tota 11 days ago

Wilson Snyder wrote:

Hi, thanks for your report.

I prefer to keep it backward compatible, so I've committed fixing this as a documentation change, removing def_exists and documenting def_params. Note this also matches up as def_params calls the option handler's defparam.

Ok, thanks for the fix. I just noticed you made the changes. However, def_params is slightly different from def_exists. The docs should read like this:

Called to determine if the define exists and the parameters it expects.
Return undef if the define doesn't exist, 0 if the define exists with no arguments,
or argument list with leading parenthesis if the define has arguments. 
Defaults to use options object's defparams method.

#4 Updated by Wilson Snyder 11 days ago

Thanks for the rewording, pushed to git.

Also available in: Atom