8000 Preprocessor doesn't differentiate between VUnit check() and other procedures of the same name · Issue #200 · VUnit/vunit · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Preprocessor doesn't differentiate between VUnit check() and other procedures of the same name #200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rwmsmith opened this issue Oct 18, 2016 · 7 comments

Comments

@rwmsmith
Copy link

The VUnit pre-processor adds line number and filename arguments to the check() procedure calls. Unfortunately, it doesn't discriminate between VUnit's check() procedures and any other procedure / function of the same name, so it adds these parameters to all such procedures. This then causes compilation failures.
The same happens with log(), which in my case clashes with a function to calculate logarithms.

@LarsAsplund
Copy link
Collaborator
LarsAsplund commented Oct 18, 2016

The enable_location_preprocessing has an optional argument additional_subprograms which allows you to add a list of custom subprograms developed to handle the extra line_num and file_name parameters. The easiest extension that will handle your problem is to add an exclude_subprograms that will take a list of subprograms to exclude, ['check', 'log'] in you case. I will add that.

You will do fine without log since you can use verbose, debug, info, warning and some more which are equivalent to a log with the log_level parameter set to the name of those procedure calls.

You can actually work around the lack of check as well since check_true will do the same thing. However, if you find a value in the optional location preprocessor and has control of the other check procedure you should consider adding the missing parameters to that.

Note that the value of location preprocessing for check type of subprograms is lost if you are setup to stop the simulation on the first failing check. In that case the call stack will reveal the location of the call and how you got there. Location for logs still has a value.

Finally, there is also an undocumented add_preprocessor method that allows you to add your own preprocessor. Maybe there is a way to distinguish VUnit checks from your checks such that the latter is left untouch by the preprocessor. I will improve the API documentation to show this method as well.

@felixn
Copy link
Contributor
felixn commented Oct 18, 2016

The same thing happens with slice assignments to signals with certain names.
For example, the following leads to compilation failures:

signal debug: std_logic_vector(31 downto 0);
debug(31 downto 16) <= X"BABE";

@LarsAsplund
Copy link
Collaborator

@felixn The preprocessor has no real parser but uses simple regexps so there will be things that we cannot handle nicely. A better parser is something for the future (#174). However, your problem case seems like something that can be fixed with regexps so I will look into that.

@kraigher
Copy link
Collaborator

A better parser is not enough. There also needs to be a semantic analys step where the function/procedure call is bound to a specific implementation based on type inference and scope.
This is a very big undertaking and maintainance burden to develop.

@kraigher
Copy link
Collaborator

An alternative solution would be to have a separate preprocessor where the user would have to provide a separate location argument themselves such as:

-- vunit_loc is a constant in a package, vunit_loc is replaced with the location information.
check(condition, vunit_loc);

The preprocessing could be more robust since the vunit_loc would be less likely to collide with anything else. The vunit_loc could be replaced with a simple unique integer by the preprocessor and there could be a package loading a generated file mapping all unique location integers to file name and line number information. If no preprocessing occured vunit_loc could be a constant that refers to an undefined location.

@LarsAsplund
Copy link
Collaborator

@kraigher Yes, there has to be a balance between effort and added value and I feel that the two suggested improvements would add value with little effort. If we were to add proper parsing and semantic analysis in the future we could take this a step further but I think it safe to say that such additions wouldn't be driven by this functionality due to the high effort. It will be other values driving such development.

Adding vunit_loc is an interesting idea but it goes against one of the original ideas to support the lazy programmer that don't want to spend time on creating unique messages and/or IDs (using the src parameter) to locate logs. It also raises a number of extra issues around how the new parameter should be added and backward compatibility for the current method. Using an end of the line pragma to tag the line for location preprocessing is another approach with somewhat different implications.

I would prefer to do the quicker fixes suggested above, leave this issue open and let this second idea mature for a while. Maybe there will be some more user input now that this issue has been opened.

LarsAsplund added a commit that referenced this issue Oct 20, 2016
…Improved preprocessor to avoid assignments to array slices sharing name with a log or check subprogram. Fixed some lint issues. Related to #200
@LarsAsplund
Copy link
Collaborator

@rwmsmith @felixn I pushed a new release addressing your issues. The updated API for enable_location_preprocessing can be found here.

I haven't address aggregate assignments so something like

(debug(31 downto 16), debug(15 downto 0)) := std_logic_vector'(x"deadbeef");

would still cause problems. For these cases you'll have to break the aggregate (or exclude debug from preprocessing).

I'll leave the issue open for a while to see if there are other issues concerning the preprocessing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants
0