Fix several ICG class template bugs #1871
Conversation
ninotarantino
force-pushed
the
1169-io_src-templates
branch
from
ab4c0e9 to
23bead3
Compare
- Fixed an issue where io_src code for some templates was being placed in the wrong file. - Fixed an issue with templates being instantiated with templates as parameters. - Fixed an issue with templates that instantiate templates. Added additional tests to cover these cases.
ninotarantino
force-pushed
the
1169-io_src-templates
branch
from
23bead3 to
47df1a2
Compare
|
@hchen99 This is ready for review. This addresses the issues with templates in io_src code I was seeing locally. It also addresses a few other related issues. It still doesn't fix #1860, though. There was a spot in I also added a new function that saves header dependencies for template arguments, then writes those additional |
|
@ninotarantino Sounds great, thank you! Will review |
| template_spec_cvis.get_class_data()->setName(in_name) ; | ||
| template_spec_cvis.get_class_data()->setFileName(fdes->getFileName()) ; |
There was a problem hiding this comment.
Any particular reason to remove this line? Seems working either way. Just wondering
There was a problem hiding this comment.
I probably put that in before I got the new includes working. That fixed the issue in #540 where the io_src was being put in the wrong file. But yeah it looks like this should work with or without this line now.
For #1860, it seems like the header dependencies template arguments are already obtained for a ClassValue, however, PrintFileContents10 might print other ClassValue earlier than the one that has header dependencies info for the template arguments. So, the dependencies set for template arguments also needs to be kept in PrintFileContents10 that they can be print at the end of printIOHeader. |
|
@ninotarantino I did a quick test using a map to store header template argument dependencies for each applicable header in PrintFileContents10, replacing the set previously stored in ClassValue. The header dependency set is printed in the same location as before in PrintFileContents10, and it seems to work, including for the case in #1860. Also added logic to avoid including the same dependency multiple times. You probably have a different approach in mind. Thought to just let you know. |
|
Oh awesome! Yeah that sounds like a better solution since it'll keep the headers at the top of the file every time. I have no preference personally, just wanted to get a working prototype for my branch. I probably won't be able to focus on this for a while so if you want to make those changes that's fine with me! Or I can do it when I have some time again. |
Understand. No problem and thanks for making this PR. I’ll go ahead and make the update to your branch. When you have time, feel free to test it to make sure it works with your sim—I might’ve missed something. |
…ues to PrintFileContents10 so they can be printed to the io_ file regardless of the order in which class info is printed.
…ues to PrintFileContents10 so they can be printed to the io_ file regardless of the order in which class info is printed. Moved the list of template argument header dependencies from ClassValues to PrintFileContents10 so they can be printed to the io_ file regardless of the order in which class info is printed.
|
@ninotarantino When you have a chance, it’d be great if you could test it with your sim and let me know if you notice anything. Some of your code was commented out during relocation in case still needed and can be deleted once tested. |
|
@hchen99 My sim still works with your changes! Everything looks good. |
… some commented code. Added support to handle template enum class type argument and removed some commented code.
Closes #1001, #540, #1169