Update all template formatting - #505
Conversation
25b463f to
29d6d5d
Compare
d3fa387 to
45cf5d5
Compare
Fixed extraneous divergence Fixed extraneous divergence
45cf5d5 to
618fb79
Compare
pelesh
left a comment
pelesh
left a comment
There was a problem hiding this comment.
Some preliminary comments:
- This PR changes the scope of scalar type aliases from private to public. This leads to an unintuitive code like
void function<scalar_type>(ScalarT x);. - There is minor scope creep as type aliases other than scalar types were handled in a similar way. What is the motivation for that?
| public: | ||
| using RealT = GridKit::ScalarTraits<ScalarT>::RealT; | ||
| using ScalarT = scalar_type; | ||
| using IdxT = index_type; | ||
| using RealT = GridKit::ScalarTraits<ScalarT>::RealT; |
There was a problem hiding this comment.
Why are these public? Shouldn't these be private as they are intended for use inside the class?
There was a problem hiding this comment.
The intent is to make a consistent use of interface type aliases across all classes. Unless there is a good reason to hide these names from consumer contexts, the interface should be the same.
| * @tparam Policy - Memory management policy (vendor specific) | ||
| * @tparam policy - Memory management policy (vendor specific) | ||
| * | ||
| * @author Slaven Peles <peless@ornl.gov> | ||
| */ | ||
| template <class Policy> | ||
| template <typename policy> | ||
| class MemoryUtils | ||
| { | ||
| public: | ||
| using Policy = policy; | ||
|
|
There was a problem hiding this comment.
The issue is about consistent template parameter formatting. This is to follow the new guidelines.
|
|
||
| /// Parameters enum | ||
| using Parameters = parameters_type; | ||
| /// Buses enum | ||
| using Buses = buses_type; | ||
| /// Signal inputs enum | ||
| using SignalInputs = signal_inputs_type; | ||
| /// Signal outputs enum | ||
| using SignalOutputs = signal_outputs_type; | ||
| /// Monitorable variables enum | ||
| using MonitorableVariables = monitorable_variables_type; | ||
|
|
There was a problem hiding this comment.
Why is this necessary? I thought the issue was with scalar types?
There was a problem hiding this comment.
This issue is about consistent template parameter format across all templates. ScalarT was used as an example, but the reasoning applies everywhere. Consumer contexts often need access to type aliases defined by the classes being used. It is rare that type aliases need to be restricted to be private to a class implementation.
Co-authored-by: pelesh <peless@ornl.gov>
2 participants
Description
Update templates everywhere to use new style guidelines
Closes #366
Checklist
-Wall -Wpedantic -Wconversion -Wextra.