remove usage of typet::subtype() #6599

kroening · 2022-01-20T10:27:34Z

This commit replaces invocations of typet::subtype() by the appropriate
methods offered by the more specific types, e.g., pointer_typet,
array_typet, and so on.

Each commit message has a non-empty body, explaining why the change was made.
n/a Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
n/a My commit message includes data points confirming performance improvements (if claimed).
My PR is restricted to a single feature or bugfix.
n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

tautschnig · 2022-01-20T10:33:02Z

src/analyses/does_remove_const.cpp

+      to_pointer_type(*target_type).base_type(), source_type->subtype());
    // We have a pointer to something, but the thing it is pointing to can't be
    // modified normally, but can through this pointer
    if(!direct_subtypes_at_least_as_const)
      return false;
    // Check the subtypes if they are pointers
-    target_type=&target_type->subtype();
+    target_type = &to_pointer_type(*target_type).base_type();


May I suggest a temporary to avoid the repeated to_pointer_type(*target_type) in this code branch?

tautschnig · 2022-01-20T10:33:27Z

src/analyses/does_remove_const.cpp

 #include <util/base_type.h>
+#include <util/pointer_expr.h>
+#include <util/std_code.h>
+#include <util/type.h>


Nit pick: I'm pretty sure util/type.h does not need to be included explicitly here.

tautschnig · 2022-01-20T10:35:19Z

src/analyses/uncaught_exceptions_analysis.cpp

 {
  PRECONDITION(type.id()==ID_pointer);

-  if(type.subtype().id() == ID_struct_tag)
-    return to_struct_tag_type(type.subtype()).get_identifier();
+  if(to_pointer_type(type).base_type().id() == ID_struct_tag)
+    return to_struct_tag_type(to_pointer_type(type).base_type())
+      .get_identifier();


Could we please change the interface here to just take a const pointer_typet &?

tautschnig · 2022-01-20T10:37:54Z

src/ansi-c/c_typecast.cpp

  if(src_type_id==ID_c_bit_field)
-    return check_c_implicit_typecast(src_type.subtype(), dest_type);
+    return check_c_implicit_typecast(
+      to_c_bit_field_type(src_type).underlying_type(), dest_type);


Nit pick: would appreciate a pair of { ... } around the multi-line body.

tautschnig · 2022-01-20T10:38:02Z

src/ansi-c/c_typecast.cpp

  if(dest_type.id()==ID_c_bit_field)
-    return check_c_implicit_typecast(src_type, dest_type.subtype());
+    return check_c_implicit_typecast(
+      src_type, to_c_bit_field_type(dest_type).underlying_type());


Nit pick: would appreciate a pair of { ... } around the multi-line body.

tautschnig · 2022-01-20T10:48:36Z

src/cpp/cpp_typecheck_constructor.cpp

+    if(
+      to_reference_type(parameter1_type).base_type().get(ID_identifier) !=
+      symbol.name)
      continue;


Nit pick: would appreciate a pair of { ... } given the multi-line condition.

tautschnig · 2022-01-20T10:48:52Z

src/cpp/cpp_typecheck_constructor.cpp

+    if(
+      to_reference_type(arg1_type).base_type().get(ID_identifier) !=
+      symbol.name)
      continue;


Nit pick: would appreciate a pair of { ... } given the multi-line condition.

tautschnig · 2022-01-20T10:53:27Z

src/goto-programs/remove_const_function_pointers.cpp

+  if(
+    type.id() == ID_array &&
+    to_array_type(type).element_type().get_bool(ID_C_constant))
    return true;

  return type.get_bool(ID_C_constant);


Nit pick: would appreciate a pair of { ... } given the multi-line condition. Or perhaps just rewrite this function to just have a return statement.

tautschnig · 2022-01-20T10:54:52Z

src/solvers/flattening/boolbv_with.cpp

+    if(
+      expr.type().id() == ID_array &&
+      boolbv_width(to_array_type(expr.type()).element_type()) != 0)
      return bvt();


Nit pick: would appreciate a pair of { ... } given the multi-line condition.

tautschnig · 2022-01-20T10:55:31Z

src/solvers/smt2/smt2_conv.cpp

+      if(to_pointer_type(expr.type()).base_type().id() == ID_empty)
      {
        // This is a gcc extension.
        // https://gcc.gnu.org/onlinedocs/gcc-4.8.0/gcc/Pointer-Arith.html
        element_size = 1;
      }
      else
      {
-        auto element_size_opt = pointer_offset_size(expr.type().subtype(), ns);
+        auto element_size_opt =
+          pointer_offset_size(to_pointer_type(expr.type()).base_type(), ns);


A temporary may be useful here to avoid the repeated to_pointer_type.

codecov · 2022-01-20T14:34:51Z

Codecov Report

Merging #6599 (3b27b76) into develop (57e7b2c) will decrease coverage by 0.00%.
The diff coverage is 72.59%.

@@             Coverage Diff             @@
##           develop    #6599      +/-   ##
===========================================
- Coverage    76.65%   76.64%   -0.01%     
===========================================
  Files         1579     1579              
  Lines       181294   181480     +186     
===========================================
+ Hits        138973   139098     +125     
- Misses       42321    42382      +61

Impacted Files	Coverage Δ
src/analyses/goto_rw.cpp	`52.30% <0.00%> (-0.13%)`	⬇️
src/analyses/uncaught_exceptions_analysis.h	`100.00% <ø> (ø)`
src/ansi-c/ansi_c_parser.cpp	`88.39% <0.00%> (ø)`
src/cpp/cpp_declarator_converter.h	`100.00% <ø> (ø)`
src/cpp/cpp_exception_id.cpp	`0.00% <0.00%> (ø)`
src/cpp/cpp_template_type.h	`64.28% <0.00%> (-17.54%)`	⬇️
src/cpp/cpp_typecheck.cpp	`79.22% <0.00%> (ø)`
src/cpp/cpp_typecheck_initializer.cpp	`29.09% <0.00%> (-0.36%)`	⬇️
src/goto-programs/interpreter.cpp	`52.29% <0.00%> (ø)`
src/goto-programs/interpreter_evaluate.cpp	`33.77% <0.00%> (-0.06%)`	⬇️
... and 89 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59155f9...3b27b76. Read the comment docs.

This commit replaces invocations of typet::subtype() by the appropriate methods offered by the more specific types, e.g., pointer_typet, array_typet, and so on.

kroening added the cleanup label Jan 20, 2022

kroening force-pushed the typet_subtype_const branch from 12497e2 to 796d841 Compare January 20, 2022 10:51

tautschnig approved these changes Jan 20, 2022

View reviewed changes

kroening force-pushed the typet_subtype_const branch 4 times, most recently from b22be30 to f87a099 Compare January 20, 2022 13:09

kroening marked this pull request as ready for review January 20, 2022 14:30

kroening requested review from chrisr-diffblue, martin-cs, NlightNFotis, peterschrammel, pkesseli, romainbrenguier, TGWDB and thomasspriggs as code owners January 20, 2022 14:31

kroening assigned peterschrammel Jan 20, 2022

kroening force-pushed the typet_subtype_const branch 2 times, most recently from ad15521 to 19c3981 Compare January 20, 2022 15:22

peterschrammel approved these changes Jan 31, 2022

View reviewed changes

remove usage of typet::subtype()

3b27b76

This commit replaces invocations of typet::subtype() by the appropriate methods offered by the more specific types, e.g., pointer_typet, array_typet, and so on.

kroening force-pushed the typet_subtype_const branch from 19c3981 to 3b27b76 Compare January 31, 2022 09:50

kroening merged commit 637f138 into develop Jan 31, 2022

kroening deleted the typet_subtype_const branch January 31, 2022 14:36

remove usage of typet::subtype() #6599

remove usage of typet::subtype() #6599

Uh oh!

Conversation

kroening commented Jan 20, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

codecov bot commented Jan 20, 2022 •

edited

Loading