Merge "header-abi-diff: Add a flag to consider opaque types with different names structurally unequal."
am: 73ea62cf25
Change-Id: I26bc6587e730fad442fda69b88aa368dee30d099
This commit is contained in:
@@ -360,7 +360,9 @@ bool HeaderAbiDiff::DumpDiffElements(
|
||||
}
|
||||
abi_diff_wrappers::DiffWrapper<T> diff_wrapper(old_element, new_element,
|
||||
ir_diff_dumper, old_types,
|
||||
new_types, &type_cache_);
|
||||
new_types,
|
||||
diff_policy_options_,
|
||||
&type_cache_);
|
||||
if (!diff_wrapper.DumpDiff(diff_kind)) {
|
||||
llvm::errs() << "Failed to diff elements\n";
|
||||
return false;
|
||||
|
||||
@@ -20,6 +20,7 @@
|
||||
#include <vector>
|
||||
|
||||
using abi_util::AbiElementMap;
|
||||
using abi_util::DiffPolicyOptions;
|
||||
|
||||
class HeaderAbiDiff {
|
||||
public:
|
||||
@@ -27,12 +28,15 @@ class HeaderAbiDiff {
|
||||
const std::string &old_dump, const std::string &new_dump,
|
||||
const std::string &compatibility_report,
|
||||
const std::set<std::string> &ignored_symbols,
|
||||
const DiffPolicyOptions &diff_policy_options,
|
||||
bool check_all_apis, abi_util::TextFormatIR text_format_old,
|
||||
abi_util::TextFormatIR text_format_new,
|
||||
abi_util::TextFormatIR text_format_diff)
|
||||
: lib_name_(lib_name), arch_(arch), old_dump_(old_dump),
|
||||
new_dump_(new_dump), cr_(compatibility_report),
|
||||
ignored_symbols_(ignored_symbols), check_all_apis_(check_all_apis),
|
||||
ignored_symbols_(ignored_symbols),
|
||||
diff_policy_options_(diff_policy_options),
|
||||
check_all_apis_(check_all_apis),
|
||||
text_format_old_(text_format_old), text_format_new_(text_format_new),
|
||||
text_format_diff_(text_format_diff) { }
|
||||
|
||||
@@ -134,6 +138,7 @@ class HeaderAbiDiff {
|
||||
const std::string &new_dump_;
|
||||
const std::string &cr_;
|
||||
const std::set<std::string> &ignored_symbols_;
|
||||
const DiffPolicyOptions &diff_policy_options_;
|
||||
bool check_all_apis_;
|
||||
std::set<std::string> type_cache_;
|
||||
abi_util::TextFormatIR text_format_old_;
|
||||
|
||||
@@ -42,9 +42,10 @@ class DiffWrapper : public AbiDiffHelper {
|
||||
abi_util::IRDiffDumper *ir_diff_dumper,
|
||||
const AbiElementMap<const abi_util::TypeIR *> &old_types,
|
||||
const AbiElementMap<const abi_util::TypeIR *> &new_types,
|
||||
const abi_util::DiffPolicyOptions &diff_policy_options,
|
||||
std::set<std::string> *type_cache)
|
||||
: AbiDiffHelper(old_types, new_types, type_cache, ir_diff_dumper),
|
||||
oldp_(oldp), newp_(newp) { }
|
||||
: AbiDiffHelper(old_types, new_types, diff_policy_options, type_cache,
|
||||
ir_diff_dumper), oldp_(oldp), newp_(newp) { }
|
||||
|
||||
bool DumpDiff(abi_util::IRDiffDumper::DiffKind diff_kind);
|
||||
|
||||
|
||||
@@ -85,6 +85,14 @@ static llvm::cl::opt<bool> allow_unreferenced_changes(
|
||||
" APIs."),
|
||||
llvm::cl::Optional, llvm::cl::cat(header_checker_category));
|
||||
|
||||
static llvm::cl::opt<bool> consider_opaque_types_different(
|
||||
"consider-opaque-types-different",
|
||||
llvm::cl::desc("Consider opaque types with different names as different"
|
||||
" .This should not be used while comparing C++ library"
|
||||
" ABIs"),
|
||||
llvm::cl::Optional, llvm::cl::cat(header_checker_category));
|
||||
|
||||
|
||||
static llvm::cl::opt<abi_util::TextFormatIR> text_format_old(
|
||||
"text-format-old", llvm::cl::desc("Specify text format of old abi dump"),
|
||||
llvm::cl::values(clEnumValN(abi_util::TextFormatIR::ProtobufTextFormat,
|
||||
@@ -142,9 +150,11 @@ int main(int argc, const char **argv) {
|
||||
if (llvm::sys::fs::exists(ignore_symbol_list)) {
|
||||
ignored_symbols = LoadIgnoredSymbols(ignore_symbol_list);
|
||||
}
|
||||
abi_util::DiffPolicyOptions diff_policy_options(
|
||||
consider_opaque_types_different);
|
||||
HeaderAbiDiff judge(lib_name, arch, old_dump, new_dump, compatibility_report,
|
||||
ignored_symbols, check_all_apis, text_format_old,
|
||||
text_format_new, text_format_diff);
|
||||
ignored_symbols, diff_policy_options, check_all_apis,
|
||||
text_format_old, text_format_new, text_format_diff);
|
||||
|
||||
abi_util::CompatibilityStatusIR status = judge.GenerateCompatibilityReport();
|
||||
|
||||
|
||||
@@ -19,6 +19,8 @@ enum DiffStatus {
|
||||
// There was a diff found, however it need not be added as a part of a diff
|
||||
// message, since it would have already been noted elsewhere.
|
||||
indirect_diff = 2,
|
||||
|
||||
opaque_diff = 3,
|
||||
};
|
||||
|
||||
static inline DiffStatus operator| (DiffStatus f,DiffStatus s) {
|
||||
@@ -46,16 +48,24 @@ struct GenericFieldDiffInfo {
|
||||
|
||||
std::string Unwind(const std::deque<std::string> *type_queue);
|
||||
|
||||
struct DiffPolicyOptions {
|
||||
DiffPolicyOptions(bool consider_opaque_types_different) :
|
||||
consider_opaque_types_different_(consider_opaque_types_different) { }
|
||||
bool consider_opaque_types_different_ = false;
|
||||
};
|
||||
|
||||
class AbiDiffHelper {
|
||||
public:
|
||||
AbiDiffHelper(
|
||||
const AbiElementMap<const abi_util::TypeIR *> &old_types,
|
||||
const AbiElementMap<const abi_util::TypeIR *> &new_types,
|
||||
const DiffPolicyOptions &diff_policy_options,
|
||||
std::set<std::string> *type_cache,
|
||||
abi_util::IRDiffDumper *ir_diff_dumper = nullptr,
|
||||
AbiElementMap<MergeStatus> *local_to_global_type_id_map = nullptr)
|
||||
: old_types_(old_types), new_types_(new_types),
|
||||
type_cache_(type_cache), ir_diff_dumper_(ir_diff_dumper),
|
||||
diff_policy_options_(diff_policy_options), type_cache_(type_cache),
|
||||
ir_diff_dumper_(ir_diff_dumper),
|
||||
local_to_global_type_id_map_(local_to_global_type_id_map) { }
|
||||
|
||||
DiffStatus CompareAndDumpTypeDiff(
|
||||
@@ -175,6 +185,7 @@ class AbiDiffHelper {
|
||||
protected:
|
||||
const AbiElementMap<const abi_util::TypeIR *> &old_types_;
|
||||
const AbiElementMap<const abi_util::TypeIR *> &new_types_;
|
||||
const DiffPolicyOptions &diff_policy_options_;
|
||||
std::set<std::string> *type_cache_ = nullptr;
|
||||
abi_util::IRDiffDumper *ir_diff_dumper_ = nullptr;
|
||||
AbiElementMap<MergeStatus> *local_to_global_type_id_map_ = nullptr;
|
||||
|
||||
@@ -786,6 +786,9 @@ DiffStatus AbiDiffHelper::CompareAndDumpTypeDiff(
|
||||
if (old_it == old_types_.end() || new_it == new_types_.end()) {
|
||||
TypeQueueCheckAndPop(type_queue);
|
||||
// One of the types were hidden, we cannot compare further.
|
||||
if (diff_policy_options_.consider_opaque_types_different_) {
|
||||
return DiffStatus::opaque_diff;
|
||||
}
|
||||
return DiffStatus::no_diff;
|
||||
}
|
||||
abi_util::LinkableMessageKind old_kind =
|
||||
@@ -800,6 +803,11 @@ DiffStatus AbiDiffHelper::CompareAndDumpTypeDiff(
|
||||
old_kind, type_queue, diff_kind);
|
||||
}
|
||||
TypeQueueCheckAndPop(type_queue);
|
||||
if (diff_policy_options_.consider_opaque_types_different_ &&
|
||||
diff_status == DiffStatus::opaque_diff &&
|
||||
(old_it->second->GetName() != new_it->second->GetName())) {
|
||||
return DiffStatus::direct_diff;
|
||||
}
|
||||
return diff_status;
|
||||
}
|
||||
|
||||
|
||||
@@ -123,7 +123,9 @@ MergeStatus TextFormatToIRReader::DoesUDTypeODRViolationExist(
|
||||
return MergeStatus(true, "");
|
||||
}
|
||||
std::set<std::string> type_cache;
|
||||
AbiDiffHelper diff_helper(type_graph_, addend.type_graph_, &type_cache,
|
||||
DiffPolicyOptions diff_policy_options(false) ;
|
||||
AbiDiffHelper diff_helper(type_graph_, addend.type_graph_,
|
||||
diff_policy_options, &type_cache,
|
||||
nullptr, local_to_global_type_id_map_);
|
||||
for (auto &contender_ud : it->second) {
|
||||
if (diff_helper.CompareAndDumpTypeDiff(contender_ud->GetSelfType(),
|
||||
|
||||
@@ -6,9 +6,17 @@ struct Cinner {
|
||||
int c;
|
||||
};
|
||||
|
||||
struct opaque_a;
|
||||
struct opaque_b;
|
||||
|
||||
struct Cstruct {
|
||||
int a;
|
||||
struct Cinner *b;
|
||||
#ifdef OPAQUE_STRUCT_A
|
||||
struct opaque_a *op;
|
||||
#elif OPAQUE_STRUCT_B
|
||||
struct opaque_b *op;
|
||||
#endif
|
||||
};
|
||||
|
||||
void CFunction(struct Cstruct **cstruct);
|
||||
|
||||
@@ -125,6 +125,28 @@ TEST_MODULES = [
|
||||
arch = '',
|
||||
api = 'current',
|
||||
),
|
||||
Module(
|
||||
name = 'libc_and_cpp_with_opaque_ptr_a',
|
||||
srcs = ['integration/c_and_cpp/source1.cpp',
|
||||
'integration/c_and_cpp/source2.c',
|
||||
],
|
||||
version_script = 'integration/c_and_cpp/map.txt',
|
||||
export_include_dirs = ['integration/c_and_cpp/include'],
|
||||
cflags = ['-DOPAQUE_STRUCT_A=1'],
|
||||
arch = '',
|
||||
api = 'current',
|
||||
),
|
||||
Module(
|
||||
name = 'libc_and_cpp_with_opaque_ptr_b',
|
||||
srcs = ['integration/c_and_cpp/source1.cpp',
|
||||
'integration/c_and_cpp/source2.c',
|
||||
],
|
||||
version_script = 'integration/c_and_cpp/map.txt',
|
||||
export_include_dirs = ['integration/c_and_cpp/include'],
|
||||
cflags = ['-DOPAQUE_STRUCT_B=1'],
|
||||
arch = '',
|
||||
api = 'current',
|
||||
),
|
||||
Module(
|
||||
name = 'libc_and_cpp_with_unused_struct',
|
||||
srcs = ['integration/c_and_cpp/source1.cpp',
|
||||
|
||||
@@ -146,6 +146,13 @@ class MyTest(unittest.TestCase):
|
||||
"libc_and_cpp", "libc_and_cpp_with_unused_struct", 0,
|
||||
['-check-all-apis', '-advice-only'])
|
||||
|
||||
def test_libc_and_cpp_opaque_pointer_diff(self):
|
||||
self.prepare_and_run_abi_diff_all_archs(
|
||||
"libc_and_cpp_with_opaque_ptr_a",
|
||||
"libc_and_cpp_with_opaque_ptr_b", 8,
|
||||
['-consider-opaque-types-different'], True,
|
||||
True)
|
||||
|
||||
def test_libgolden_cpp_return_type_diff(self):
|
||||
self.prepare_and_run_abi_diff_all_archs(
|
||||
"libgolden_cpp", "libgolden_cpp_return_type_diff", 8)
|
||||
|
||||
Reference in New Issue
Block a user