Skip to content
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

bugs and unnecessary code parts in hdl_writer/hdl_writer_vhdl.cpp #161

Closed
FabianAlbertRub opened this issue Jul 5, 2019 · 5 comments
Closed
Assignees
Labels
Priority: High Needs to be addressed as soon as possible Status: Available Ready for pickup Type: Bug This is a confirmed or unconfirmed bug

Comments

@FabianAlbertRub
Copy link
Contributor

  • the if-block in l. 67:
    if (std::all_of(net_temp.begin(), net_temp.end(), ::isdigit)) {...}
    can't be entered, since the get_net_name function already adds the "NET_" prefix to a net with only digits.

  • consider the following lines of code (l. 372 ff.):

    if (std::get<1>(d.first) == "loc" || type == "time")
        continue;
    if (type == "time")
    {...}
    

    The second if block can not be entered, so generic time data will not be handled correctly

  • the function get_port_name(std::string pin) is unused, so it can be removed. Or will it be used in later development?

(Since these are only small issues, which can easily fixed by only one person, i wrote them together in one report ;))

@FabianAlbertRub FabianAlbertRub added the Type: Bug This is a confirmed or unconfirmed bug label Jul 5, 2019
@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label Type: Bug to this issue, with a confidence of 0.64. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@swallat swallat added Priority: High Needs to be addressed as soon as possible Status: Available Ready for pickup labels Jul 8, 2019
@swallat swallat self-assigned this Jul 8, 2019
@swallat swallat moved this from To do to Review required in HAL – Hardware Analyzer Jul 8, 2019
@swallat swallat moved this from Review required to Reviewer approved in HAL – Hardware Analyzer Jul 8, 2019
@swallat swallat moved this from Reviewer approved to To do in HAL – Hardware Analyzer Jul 8, 2019
@nils1603
Copy link
Member

This reallyyy needs work! We need to write tests. Is there any VHDL parser that could be used to parse our written VHDL again? We could use the parser to write tests!

@RenWal
Copy link
Contributor

RenWal commented Jul 10, 2019

Can we implement basic self-testing by looping back our VHDL output to our own VHDL parser?

@FabianAlbertRub
Copy link
Contributor Author

Yeah, I've already written tests for the writer, based on comparisons with our parsers input(this thread is actually based on my testing observations).
To check the output for every call of the writer may be an interesting idea, but can result in worse run-time (especially for bigger netlists). At the other hand it seems to be a nice idea to feed the tests with more data. Maybe we can discuss this in our next meeting ;)

@nils1603
Copy link
Member

We could do that, but our parser is also excepting VHDL code that is 100% accurate regarding the specification.

HAL – Hardware Analyzer automation moved this from To do to Done Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High Needs to be addressed as soon as possible Status: Available Ready for pickup Type: Bug This is a confirmed or unconfirmed bug
Projects
Development

No branches or pull requests

5 participants