[v3] Don't ignore options when diffing translation units (.bi files).

Message ID 20200203120439.232845-1-gprocida@google.com
State Committed
Headers
Series [v3] Don't ignore options when diffing translation units (.bi files). |

Commit Message

Aleksei Vetrov via Libabigail Jan. 1, 2020, midnight UTC
  There was an inconsistency in the way the diff context was used for
different file types. This change eliminates this and so .bi files now
have all the command line options applied to their diffs.

	* tests/data/Makefile.am: Add test case files.
	* tests/data/test-abidiff-exit/test-loc-*: New test cases.
	* tests/test-abidiff-exit.cc: Add new test cases.
	* tools/abidiff.cc: Use populated ctxt for translation unit
	diff.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 tests/data/Makefile.am                         |  4 ++++
 tests/data/test-abidiff-exit/test-loc-v0.bi    | 13 +++++++++++++
 tests/data/test-abidiff-exit/test-loc-v1.bi    | 13 +++++++++++++
 .../test-loc-with-locs-report.txt              | 12 ++++++++++++
 .../test-loc-without-locs-report.txt           | 12 ++++++++++++
 tests/test-abidiff-exit.cc                     | 18 ++++++++++++++++++
 tools/abidiff.cc                               |  3 +--
 7 files changed, 73 insertions(+), 2 deletions(-)
 create mode 100644 tests/data/test-abidiff-exit/test-loc-v0.bi
 create mode 100644 tests/data/test-abidiff-exit/test-loc-v1.bi
 create mode 100644 tests/data/test-abidiff-exit/test-loc-with-locs-report.txt
 create mode 100644 tests/data/test-abidiff-exit/test-loc-without-locs-report.txt
  

Comments

Dodji Seketeli Jan. 1, 2020, midnight UTC | #1
Hello Giuliano,

Giuliano Procida <gprocida@google.com> a ?crit:

> There was an inconsistency in the way the diff context was used for
> different file types. This change eliminates this and so .bi files now
> have all the command line options applied to their diffs.
>
> 	* tests/data/Makefile.am: Add test case files.
> 	* tests/data/test-abidiff-exit/test-loc-*: New test cases.
> 	* tests/test-abidiff-exit.cc: Add new test cases.
> 	* tools/abidiff.cc: Use populated ctxt for translation unit
> 	diff.

This looks great to me, so I have applied it to master.

I have just slightly edited the changelog part to mention that the the
name of the function that got modified in abidiff.cc and the name of the
variable that got modified in test-abidiff-exit.cc.  These are picking
of nits on my part.

Thanks for catching this problem!

Cheers,
  

Patch

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 1ae9d31a..1f574d2e 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -103,6 +103,10 @@  test-abidiff-exit/test2-filtered-removed-fns-v1.c \
 test-abidiff-exit/test2-filtered-removed-fns-v0.o \
 test-abidiff-exit/test2-filtered-removed-fns-v1.o \
 test-abidiff-exit/test2-filtered-removed-fns.abignore \
+test-abidiff-exit/test-loc-v0.bi \
+test-abidiff-exit/test-loc-v1.bi \
+test-abidiff-exit/test-loc-with-locs-report.txt \
+test-abidiff-exit/test-loc-without-locs-report.txt \
 \
 test-diff-dwarf/test0-v0.cc		\
 test-diff-dwarf/test0-v0.o			\
diff --git a/tests/data/test-abidiff-exit/test-loc-v0.bi b/tests/data/test-abidiff-exit/test-loc-v0.bi
new file mode 100644
index 00000000..da3a7c64
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-loc-v0.bi
@@ -0,0 +1,13 @@ 
+<abi-corpus path='test-loc-v0.o' architecture='elf-amd-x86_64'>
+  <elf-function-symbols>
+    <elf-symbol name='x' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+  </elf-function-symbols>
+  <abi-instr version='1.0' address-size='64' path='test-loc-v0.c' comp-dir-path='/tmp' language='LANG_C99'>
+    <type-decl name='unsigned char' size-in-bits='8' id='type-id-1'/>
+    <type-decl name='void' id='type-id-2'/>
+    <function-decl name='x' mangled-name='x' filepath='test-loc-v0.c' line='1' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='x'>
+      <parameter type-id='type-id-1' name='y' filepath='test-loc-v0.c' line='1' column='1'/>
+      <return type-id='type-id-2'/>
+    </function-decl>
+  </abi-instr>
+</abi-corpus>
diff --git a/tests/data/test-abidiff-exit/test-loc-v1.bi b/tests/data/test-abidiff-exit/test-loc-v1.bi
new file mode 100644
index 00000000..43ac6ecd
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-loc-v1.bi
@@ -0,0 +1,13 @@ 
+<abi-corpus path='test-loc-v1.o' architecture='elf-amd-x86_64'>
+  <elf-function-symbols>
+    <elf-symbol name='x' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+  </elf-function-symbols>
+  <abi-instr version='1.0' address-size='64' path='test-loc-v1.c' comp-dir-path='/tmp' language='LANG_C99'>
+    <type-decl name='signed char' size-in-bits='8' id='type-id-1'/>
+    <type-decl name='void' id='type-id-2'/>
+    <function-decl name='x' mangled-name='x' filepath='test-loc-v1.c' line='1' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='x'>
+      <parameter type-id='type-id-1' name='y' filepath='test-loc-v1.c' line='1' column='1'/>
+      <return type-id='type-id-2'/>
+    </function-decl>
+  </abi-instr>
+</abi-corpus>
diff --git a/tests/data/test-abidiff-exit/test-loc-with-locs-report.txt b/tests/data/test-abidiff-exit/test-loc-with-locs-report.txt
new file mode 100644
index 00000000..d62e405e
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-loc-with-locs-report.txt
@@ -0,0 +1,12 @@ 
+Functions changes summary: 0 Removed, 1 Changed, 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+1 function with some indirect sub-type change:
+
+  [C]'function void x(unsigned char)' at test-loc-v1.c:1:1 has some indirect sub-type changes:
+    parameter 1 of type 'unsigned char' changed:
+      type name changed from 'unsigned char' to 'signed char'
+      type size hasn't changed
+
+
+
diff --git a/tests/data/test-abidiff-exit/test-loc-without-locs-report.txt b/tests/data/test-abidiff-exit/test-loc-without-locs-report.txt
new file mode 100644
index 00000000..5a72047e
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-loc-without-locs-report.txt
@@ -0,0 +1,12 @@ 
+Functions changes summary: 0 Removed, 1 Changed, 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+1 function with some indirect sub-type change:
+
+  [C]'function void x(unsigned char)' has some indirect sub-type changes:
+    parameter 1 of type 'unsigned char' changed:
+      type name changed from 'unsigned char' to 'signed char'
+      type size hasn't changed
+
+
+
diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index e6df5933..4cef727e 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -93,6 +93,24 @@  InOutSpec in_out_specs[] =
     "data/test-abidiff-exit/test2-filtered-removed-fns-report1.txt",
     "output/test-abidiff-exit/test2-filtered-removed-fns-report1.txt"
   },
+  {
+    "data/test-abidiff-exit/test-loc-v0.bi",
+    "data/test-abidiff-exit/test-loc-v1.bi",
+    "",
+    "",
+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+    "data/test-abidiff-exit/test-loc-with-locs-report.txt",
+    "output/test-abidiff-exit/test-loc-with-locs-report.txt"
+  },
+  {
+    "data/test-abidiff-exit/test-loc-v0.bi",
+    "data/test-abidiff-exit/test-loc-v1.bi",
+    "",
+    "--no-show-locs",
+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+    "data/test-abidiff-exit/test-loc-without-locs-report.txt",
+    "output/test-abidiff-exit/test-loc-without-locs-report.txt"
+  },
   {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
 };
 
diff --git a/tools/abidiff.cc b/tools/abidiff.cc
index 1d02f65e..32f793e8 100644
--- a/tools/abidiff.cc
+++ b/tools/abidiff.cc
@@ -1243,8 +1243,7 @@  main(int argc, char* argv[])
 
       if (t1)
 	{
-	  diff_context_sptr diff_ctxt(new diff_context);
-	  translation_unit_diff_sptr diff = compute_diff(t1, t2, diff_ctxt);
+	  translation_unit_diff_sptr diff = compute_diff(t1, t2, ctxt);
 	  if (diff->has_changes())
 	    diff->report(cout);
 	}