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

Codebase: Macro to iterate over nodes of known type #1304

Closed
veripoolbot opened this issue Apr 18, 2018 · 4 comments
Closed

Codebase: Macro to iterate over nodes of known type #1304

veripoolbot opened this issue Apr 18, 2018 · 4 comments

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Apr 18, 2018


Author Name: Stefan Wallentowitz (@wallento)
Original Redmine Issue: 1304 from https://www.veripool.org


There is a pattern in the codebase when a list of nodes is iterated and the type of each of the nodes is equal and known. This is for example to iterate over lists of pins, etc.

This introduces a macro ASTNODE_ITERATE that has three parameters:

  • type: Is the type of the nodes (Ast is the matching class name)

  • it: Name of the iterator variable

  • init: Initialization statement for iterator

All occurrences of this pattern have been accordingly replaced:

 for (Ast<type> *<it> = <init>; <it>; <it>=<it>->nextp()->cast<type>())

This improves readability and some time in the future this may be a good marker to replace data containers or similar.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Apr 18, 2018


Original Redmine Comment
Author Name: Stefan Wallentowitz (@wallento)
Original Date: 2018-04-18T14:48:37Z


Sorry, I copied the commit text and now found how strange it looks here.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Apr 19, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-04-19T13:57:23Z


I need to think if I like this.

However please use the develop-v4 branch which will become 4.000 for any large-touch wholesale changes. Even more so as that removes the cast methods.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Apr 19, 2018


Original Redmine Comment
Author Name: Stefan Wallentowitz (@wallento)
Original Date: 2018-04-19T14:01:00Z


Thanks for pointing this out, I was not aware of this branch so far.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented May 6, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-05-06T14:26:08Z


For the time being I don't think we want to change this, but I appreciate your efforts to clean up and in general expect to accept other cleanups.

There's two main concerns. First, it makes the code harder to read for people not familiar with the code - if there was thousands it might be worth it but not for only 58. Second, in a few years once C++11 compilers are common (not yet), we can use the C++11 for loop syntax ("for (AstFoo* itp : nodep->itemsp()") for this and a lot of other things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.