On 05/09/2017 10:07 AM, David Malcolm wrote:
+static const char *
+type_to_string_with_compare (tree type, tree peer, bool verbose,
+ bool show_color)
+{
+ for (int idx = 0; idx < len_max; idx++)
+ {
...
+ if (idx + 1 < len_max)
+ pp_character (pp, ',');
+ }
My favorite idiom for this is to place the test at the beginning of the
loop and avoid an extra loop value check. (perhaps optimizers are smart
enough to tell that 'idx + 1 < max' and 'idx++, idx < max' are the same
these days)
if (idx)
pp_character (pp, ',');
+static void
+print_template_tree_comparison (pretty_printer *pp, tree type_a, tree type_b,
+ bool verbose, int indent)
It looks to me that type_to_string_with_compare and
print_template_tree_comparison are doing very nearly the same thing,
with a little formatting difference. How hard would it be to have them
forward to a worker function?
+ unsigned int chunk_idx;
+ for (chunk_idx = 0; args[chunk_idx]; chunk_idx++)
+ ;
I have a fondness for putting 'continue;' as the body of such empty
loops. Dunno if that's style-compliant though.
+void
+cxx_format_postprocessor::handle (pretty_printer *pp)
+{
+ /* If we have one of %H and %I, the other should have
+ been present. */
+ if (m_type_a.m_tree || m_type_b.m_tree)
+ {
+ gcc_assert (m_type_a.m_tree);
+ gcc_assert (m_type_b.m_tree);
+ }
+ if (m_type_a.m_tree && m_type_b.m_tree)
As you fall into this. Why not simply
if (m_type_a.m_tree || m_type_b.m_tree)
{ do stuff that will seg fault if one's null }
+ gcc_assert (type_a.m_tree);
And these asserts are confusing, because some, at least, seem to be
checking the if condition.
+ gcc_assert (type_a.m_buffer_ptr);
+ gcc_assert (type_b.m_tree);
+ gcc_assert (type_b.m_buffer_ptr);
Generally the C++ bits look good, and my style comments are FWIW not
obligatory.
1) is it possible to commonize the two functions I mention
2) cleanup the unnecessary asserts in cxx_format_postprocessor::handle
non-c++ bits not reviewed.
nathan
--
Nathan Sidwell
