Enable -Wconversion warnings to avoid inadvertent lowering conversions

[leekillough]

This adds -Wconversion and warns (as an error with -Werror) when types are narrowly converted without explicit casts. For example, if a uint16_t struct member is set from a uint32_t RISC-V instruction decode expression without explicitly casting it to uint16_t.

A few class member and function parameter types were changed to minimize casts; source code changes and explicit casts were minimized.

[kpgriesser]

One change needed to fix compile on MacOS.

diff --git a/src/RevTracer.cc b/src/RevTracer.cc
index 32355ff..636fbf2 100644
--- a/src/RevTracer.cc
+++ b/src/RevTracer.cc
@@ -149,11 +149,11 @@ bool RevTracer::OutputOK() {
   return outputEnabled || events.f.trc_ctl;
 }
 
-void RevTracer::regRead( uint64_t r, uint64_t v ) {
+void RevTracer::regRead( size_t r, uint64_t v ) {
   traceRecs.emplace_back( TraceRec_t( RegRead, r, v ) );
 }
 
-void RevTracer::regWrite( uint64_t r, uint64_t v ) {
+void RevTracer::regWrite( size_t r, uint64_t v ) {
   traceRecs.emplace_back( TraceRec_t( RegWrite, r, v ) );
 }

[leekillough]

One change needed to fix compile on MacOS.

Actually, the change needs to be in RevTracer.h:

   /// RevTracer: capture instruction to be traced
   void SetFetchedInsn( uint64_t _pc, uint32_t _insn );
   /// RevTracer: capture register read
-  void regRead( size_t r, uint64_t v );
+  void regRead( uint64_t r, uint64_t v );
   /// RevTracer: capture register write.
-  void regWrite( size_t r, uint64_t v );
+  void regWrite( uint64_t r, uint64_t v );
   /// RevTracer: capture memory write.
   void memWrite( uint64_t adr, size_t len, const void* data );
   /// RevTracer: capture memory read

There was a mismatch between the declaration and definition, which is not an error on a machine for which size_t == uint64_t. Since the TraceRec_t constructor takes uint64_t arguments, and since in some places like RevRegFile.h we call regRead() and regWrite() with uint64_t arguments (even if a register number fits in 8 bits), I changed regRead() and regWrite() to take a uint64_t r first argument, so that we wouldn't need to add casts to size_t in calls to regRead() and regWrite() for MacOS.

One thing that this comment made me notice, is that emplace_back() does not need TraceRec_t( ) to be constructed before its call -- the arguments to emplace_back() (in fact, all container "emplace" methods) are precisely the TraceRec_t constructor arguments themselves, so TraceRec_t() does not need to be wrapped around them -- that's just adding an extra move constructor to the call chain and making the code unnecessarily verbose. I have pushed a change which makes all calls to emplace_back() simpler.

So:

   traceRecs.emplace_back( TraceRec_t( RegRead, r, v ) );

can be replaced with simply:

   traceRecs.emplace_back( RegRead, r, v );

[kpgriesser]

Just curious, what does setting 'typename T = void' buy?

[leekillough]

Just curious, what does setting 'typename T = void' buy?

It declares a template taking a type parameter, which, if omitted from the function call, such as by calling GetCSRSetter(...) normally instead of calling it GetCSRSetter<uint64_t>(...), defaults that parameter to void. In this use-case we don't actually care about the type T -- we simply pass it as a template argument of make_dependent<T>(...) so that the function argument of make_dependent() is delayed evaluation until the template with parameter T is instantiated. Here are the principles it uses:

1. A template function is defined as having so many parameters of different kinds (template parameters can either be type, non-type or other template; here we're only using the type kind).
2. The template arguments corresponding to the template parameters can either be deduced by matching their appearance in actual function call arguments, explicitly defined in <...> arrow brackets at the call site, or defaulted if defaults exist in the template declaration. One of those 3 methods must define every template argument corresponding to each template parameter, or the "template argument deduction" fails.
3. Within the template definition, any expression which depends on template parameters like T is called dependent, and its evaluation and semantic correctness is deferred until the template is actually instantiated with template arguments. It is not fully evaluated or judged for anything beyond syntactic correctness at template definition time.
4. The make_dependent<T>( expr ) acts like an identity operation on expr, except that it makes the expression dependent on T so that its evaluation and semantic correctness are delayed until the template with parameter T is actually instantiated.
5. This allows expr to have an expression involving RevCore which is invalid at template definition time because RevCore is an incomplete type and we don't know about its members yet, only that it has been forward-declared as a class. But when we finally instantiate the CSR template function, RevCore will be a complete type, T will be defined by the template instantiation, and make_dependent<T>(...) can call RevCore methods because RevCore is complete by then.
6. This technique of using templates can break circular dependencies. Two classes like RevCore and RevCSR can both call methods of each other if one of the class's calls is made dependent on a template parameter and the template is not instantiated until the other class is fully defined. It's analogous to mutually recursive functions. You make a forward declaration of one class, call methods of it in another class which are deferred until template instantiation, and then actually define the class. After both classes are defined, the template can be instantiated. The dependence of expressions on template parameters breaks the circular dependency, even if the actual template argument passed to make_dependent does not change its result.

[leekillough]

See the discussion here, opened by me.

Rev has a lot of circular dependencies that I invented make_dependent<type1, type2, ...>( expr ) to break circular dependencies.

In RevOpts.cc make_dependent<decltype( var )>(...) is used to defer evaluation of an expression until a generic lambda (lambda with auto parameters)'s type parameters are known at lambda call-time. Generic lambdas create templates "under the hood" which have the same effect of deferring dependent expressions which depend on the type of the auto parameter's actual argument.

[kpgriesser]

checks pass