Code clarity/readability

Return early on problems to avoid having to remember the problem until the end of a large block and to reduce nesting
Indent TODO comments to keep them together and help IDE's to highlight them correctly
Name expressions that are used repeatedly
Remove unused parameter names
Use count instead of find if the returned value isn't used
Clarified the English in comments

Author: NathanJPhillips

jbmc/src/java_bytecode/generic_parameter_specialization_map_keys.cpp

@@ -80,40 +80,42 @@ void generic_parameter_specialization_map_keyst::insert_pairs_for_pointer(
   const pointer_typet &pointer_type,
   const typet &pointer_subtype_struct)
 {
-  if(is_java_generic_type(pointer_type))
+  if(!is_java_generic_type(pointer_type))
+    return;
+  // The supplied type must be the full type of the pointer's subtype
+  PRECONDITION(
+    pointer_type.subtype().get(ID_identifier) ==
+    pointer_subtype_struct.get(ID_name));
+
+  // If the pointer points to:
+  // - an incomplete class or
+  // - a class that is neither generic nor implicitly generic (this
+  //   may be due to unsupported class signature)
+  // then ignore the generic types in the pointer and do not add any pairs.
+  // TODO TG-1996 should decide how mocking and generics should work
+  //   together. Currently an incomplete class is never marked as generic. If
+  //   this changes in TG-1996 then the condition below should be updated.
+  if(to_java_class_type(pointer_subtype_struct).get_is_stub())
+    return;
+  if(
+    !is_java_generic_class_type(pointer_subtype_struct) &&
+    !is_java_implicitly_generic_class_type(pointer_subtype_struct))
   {
-    // The supplied type must be the full type of the pointer's subtype
-    PRECONDITION(
-      pointer_type.subtype().get(ID_identifier) ==
-      pointer_subtype_struct.get(ID_name));
+    return;
+  }
 
-    // If the pointer points to:
-    // - an incomplete class or
-    // - a class that is neither generic nor implicitly generic (this
-    //  may be due to unsupported class signature)
-    // then ignore the generic types in the pointer and do not add any pairs.
-    // TODO TG-1996 should decide how mocking and generics should work
-    // together. Currently an incomplete class is never marked as generic. If
-    // this changes in TG-1996 then the condition below should be updated.
-    if(
-      !to_java_class_type(pointer_subtype_struct).get_is_stub() &&
-      (is_java_generic_class_type(pointer_subtype_struct) ||
-       is_java_implicitly_generic_class_type(pointer_subtype_struct)))
-    {
-      const java_generic_typet &generic_pointer =
-        to_java_generic_type(pointer_type);
-      const std::vector<java_generic_parametert> &generic_parameters =
-        get_all_generic_parameters(pointer_subtype_struct);
+  const java_generic_typet &generic_pointer =
+    to_java_generic_type(pointer_type);
 
-      INVARIANT(
-        generic_pointer.generic_type_arguments().size() ==
-          generic_parameters.size(),
-        "All generic parameters of the pointer type need to be specified");
+  const std::vector<java_generic_parametert> &generic_parameters =
+    get_all_generic_parameters(pointer_subtype_struct);
+  const java_generic_typet::generic_type_argumentst &type_args =
+    generic_pointer.generic_type_arguments();
+  INVARIANT(
+    type_args.size() == generic_parameters.size(),
+    "All generic parameters of the pointer type need to be specified");
 
-      insert_pairs(
-        generic_parameters, generic_pointer.generic_type_arguments());
-    }
-  }
+  insert_pairs(generic_parameters, type_args);
 }
 
 /// Add a pair of a parameter and its types for each generic parameter of the
@@ -137,24 +139,29 @@ void generic_parameter_specialization_map_keyst::insert_pairs_for_symbol(
   // then ignore the generic types in the struct_tag_type and do not add any
   // pairs.
   // TODO TG-1996 should decide how mocking and generics should work
-  // together. Currently an incomplete class is never marked as generic. If
-  // this changes in TG-1996 then the condition below should be updated.
+  //   together. Currently an incomplete class is never marked as generic. If
+  //   this changes in TG-1996 then the condition below should be updated.
+  if(!is_java_generic_struct_tag_type(struct_tag_type))
+    return;
+  if(to_java_class_type(symbol_struct).get_is_stub())
+    return;
   if(
-    is_java_generic_struct_tag_type(struct_tag_type) &&
-    !to_java_class_type(symbol_struct).get_is_stub() &&
-    (is_java_generic_class_type(symbol_struct) ||
-     is_java_implicitly_generic_class_type(symbol_struct)))
+    !is_java_generic_class_type(symbol_struct) &&
+    !is_java_implicitly_generic_class_type(symbol_struct))
   {
-    const java_generic_struct_tag_typet &generic_symbol =
-      to_java_generic_struct_tag_type(struct_tag_type);
+    return;
+  }
+  const java_generic_struct_tag_typet &generic_symbol =
+    to_java_generic_struct_tag_type(struct_tag_type);
 
-    const std::vector<java_generic_parametert> &generic_parameters =
-      get_all_generic_parameters(symbol_struct);
+  const std::vector<java_generic_parametert> &generic_parameters =
+    get_all_generic_parameters(symbol_struct);
+  const java_generic_typet::generic_type_argumentst &type_args =
+    generic_symbol.generic_types();
 
-    INVARIANT(
-      generic_symbol.generic_types().size() == generic_parameters.size(),
-      "All generic parameters of the superclass need to be concretized");
+  INVARIANT(
+    type_args.size() == generic_parameters.size(),
+    "All generic parameters of the superclass need to be concretized");
 
-    insert_pairs(generic_parameters, generic_symbol.generic_types());
-  }
+  insert_pairs(generic_parameters, type_args);
 }

jbmc/src/java_bytecode/java_object_factory.cpp

@@ -487,8 +487,8 @@ void java_object_factoryt::gen_nondet_pointer_init(
   // If we are changing the pointer, we generate code for creating a pointer
   // to the substituted type instead
   // TODO if we are comparing array types we need to compare their element
-  // types. this is for now done by implementing equality function especially
-  // for java types, technical debt TG-2707
+  //   types. this is for now done by implementing equality function especially
+  //   for java types, technical debt TG-2707
   if(!equal_java_types(replacement_pointer_type, pointer_type))
   {
     // update generic_parameter_specialization_map for the new pointer

jbmc/src/java_bytecode/select_pointer_type.cpp

@@ -18,9 +18,8 @@ pointer_typet select_pointer_typet::convert_pointer_type(
   const pointer_typet &pointer_type,
   const generic_parameter_specialization_mapt
     &generic_parameter_specialization_map,
-  const namespacet &ns) const
+  const namespacet &) const
 {
-  (void)ns; // unused parameter
   // if we have a map of generic parameters -> types and the pointer is
   // a generic parameter, specialize it with concrete types
   if(!generic_parameter_specialization_map.empty())
@@ -51,7 +50,7 @@ pointer_typet select_pointer_typet::specialize_generics(
 
     // avoid infinite recursion by looking at each generic argument from
     // previous assignments
-    if(visited_nodes.find(parameter_name) != visited_nodes.end())
+    if(visited_nodes.count(parameter_name) != 0)
     {
       const optionalt<pointer_typet> result = get_recursively_instantiated_type(
         parameter_name, generic_parameter_specialization_map);
@@ -112,7 +111,7 @@ pointer_typet select_pointer_typet::specialize_generics(
 ///
 /// Example:
 /// `class MyGeneric<T,U> { MyGeneric<U,T> gen; T t;}`
-/// When instantiating `MyGeneric<Integer,String> my` we need to for example
+/// For example, when instantiating `MyGeneric<Integer,String> my` we need to
 /// resolve the type of `my.gen.t`. The map would in this context contain
 /// - T -> (Integer, U)
 /// - U -> (String, T)