Fix printf formats #4026
Fix printf formats #4026
Conversation
passes/memory/memory_libmap.cc
Outdated
| @@ -690,7 +690,7 @@ bool apply_clock(MemConfig &cfg, const PortVariant &def, SigBit clk, bool clk_po | |||
|
|
|||
| // Perform write port assignment, validating clock options as we go. | |||
| void MemMapping::assign_wr_ports() { | |||
| log_reject(stringf("Assigning write ports... (candidate configs: %lu)", cfgs.size())); | |||
| log_reject(stringf("Assigning write ports... (candidate configs: %zu)", cfgs.size())); | |||
There was a problem hiding this comment.
As I am reading into the standard std::vector<T>::size() returns an implementation-defined size type. Do we need to both use %zu and add an explicit cast to size_t for this to be portable?
There was a problem hiding this comment.
Can implementation-defined size type be larger than architecture-defined size type? Does C++ standard require vector to be completely in memory? If yes, then cast to size_t is safe, if not... 64-bit size_type on 32-bit CPU with 32-bit size_t... Should be unlikely enough. Even current state is better, than the one before.
There was a problem hiding this comment.
From a practical perspective, I would argue that if you have more than 4 billion possible configuration options to choose between to map your memory instance, getting the number truncated in a debug log message is the least of your problems... we could cast to uint64_t just in case, but I'm happy with anything as long as we don't cast to char.
There was a problem hiding this comment.
I will just cast to size_t
|
Did I randomy request review? |
|
Nah, it's a github feature where it automatically requests review when any file listed in CODEOWNERS is modified. (Also, IIRC you need at least triage permissions on a repo to request reviews.) |
|
@uis246 Can I force-push to your branch with the |
|
Yes. Sorry I forgot about this. |
Just what compiler found. Uses proper types like %zu for size_t and %PRIx64 for uint64_t in hexadecimal.