Move IRUtils Literal get functions to the respective IR members. Add stringliteral get function. #4623
Conversation
fruffy
added
p4tools
fruffy
force-pushed
the
fruffy/stringliteral
branch
from
002e4b2
to
f058dab
Compare
Do you mean e.g. |
Yes, something in that form. |
OK, I think that would make more sense than the freestanding functions. |
fruffy
force-pushed
the
fruffy/stringliteral
branch
from
f058dab
to
4d9d190
Compare
Alright, gave this a shot. There is probably more utils we could add. |
fruffy
force-pushed
the
fruffy/stringliteral
branch
from
4d9d190
to
333ed41
Compare
fruffy
changed the title
There was a problem hiding this comment.
I think there is a problem when caching values that can have valid source info. Either we cannot cache these at all, or we would need the cache to take the source info into account (but then it is unlikely it would hit). Otherwise, we risk breaking error messages because the programmer will believe that the source info is there, but it won't (or it will be wrong one!). I wonder if the cache makes sense at all. Maybe in testgen it could...
I do realize this is not new in the PR. But since you have changed this code and likely intend to use it more widely, I suggest this is fixed now.
fruffy
force-pushed
the
fruffy/stringliteral
branch
3 times, most recently
from
51cef18
to
3acde1b
Compare
fruffy
changed the title
fruffy
changed the title
fruffy
force-pushed
the
fruffy/stringliteral
branch
from
3acde1b
to
cde8444
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
| @@ -11,6 +11,8 @@ | |||
| #include <tuple> | |||
| #include <type_traits> | |||
|
|
|||
| #include "lib/big_int_util.h" | |||
There was a problem hiding this comment.
maybe it would make sense to reverse the dependency and instantiate hasher inside lib/big_int_util.h? Right now it seems that everyone who'd like to use lib/hash.h would pull big_int_util.h, this seems to be not quite a good thing to me.
There was a problem hiding this comment.
big_int_util is mostly used for the declaration of big_int, which is used fairly pervasively in the compiler. We can split that out?
There was a problem hiding this comment.
Right. But why does it become the hash dependency? It seems it should be the opposite: big_int_utils should depend on hash and not vice versa. To make it a bit more clear: why would #include lib/hash.h pull in the declaration of big_int?
There was a problem hiding this comment.
Yes, I have a fix for that in #4655.
I have been doing this a lot lately, so might as well contribute a utility function. Maybe all of these "get" functions should be static class members?