From e00e5f1f0909843ef76f9c20be29f4d0ac7afd41 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Thu, 7 Mar 2019 21:55:44 +0000 Subject: [PATCH] Stop ignoring uses for builtins with mappings IWYU used to ignore all uses of builtins. This makes sense normally. However, when a symbol has an explicit mapping defined, we should not ignore uses of it. In particular, for example, one might require a particular header to be included when certain builtins are used, and this change allows that by defining a mapping for that builtin. Also, under some situations some standard library functions (e.g. std::round) get treated as builtins. With this change IWYU respects mappings for such builtins, and by defining them suitably it will no longer remove #include . --- iwyu_output.cc | 20 ++++++++++++-- run_iwyu_tests.py | 2 ++ tests/cxx/built_ins_with_mapping-d1.h | 15 ++++++++++ tests/cxx/built_ins_with_mapping.cc | 40 +++++++++++++++++++++++++++ tests/cxx/built_ins_with_mapping.h | 34 +++++++++++++++++++++++ tests/cxx/built_ins_with_mapping.imp | 9 ++++++ 6 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 tests/cxx/built_ins_with_mapping-d1.h create mode 100644 tests/cxx/built_ins_with_mapping.cc create mode 100644 tests/cxx/built_ins_with_mapping.h create mode 100644 tests/cxx/built_ins_with_mapping.imp diff --git a/iwyu_output.cc b/iwyu_output.cc index 1412c5a..ba70b01 100644 --- a/iwyu_output.cc +++ b/iwyu_output.cc @@ -1111,6 +1111,11 @@ void ProcessForwardDeclare(OneUse* use, } } +// Returns true if the given symbol has a mapping defined to a file. +static bool HasMapping(const string& symbol) { + return !GlobalIncludePicker().GetCandidateHeadersForSymbol(symbol).empty(); +} + void ProcessFullUse(OneUse* use, const IwyuPreprocessorInfo* preprocessor_info) { CHECK_(use->decl() && "Must call ProcessFullUse on a decl"); @@ -1118,6 +1123,14 @@ void ProcessFullUse(OneUse* use, if (use->ignore_use()) // we're already ignoring it return; + // We normally ignore uses for builtins, but when there is a mapping defined + // for the symbol, we should respect that. So, we need to determine whether + // the symbol has any mappings. + bool is_builtin_function = IsBuiltinFunction(use->decl(), use->symbol_name()); + + bool is_builtin_function_with_mappings = + is_builtin_function && HasMapping(use->symbol_name()); + // (B1) If the definition is after the use, re-point to a prior decl. // If iwyu followed the language precisely, this wouldn't be // necessary: code wouldn't compile if a full-use didn't have the @@ -1162,7 +1175,7 @@ void ProcessFullUse(OneUse* use, // All this is moot when FunctionDecls are being defined, all their redecls // are separately registered as uses so that a definition anchors all its // declarations. - if (!use->is_function_being_defined()) { + if (!use->is_function_being_defined() && !is_builtin_function_with_mappings) { set all_redecls; if (isa(use->decl()) || isa(use->decl())) all_redecls.insert(use->decl()); // for classes, just consider the dfn @@ -1188,7 +1201,7 @@ void ProcessFullUse(OneUse* use, return; } // A compiler builtin without a predefined header file (e.g. __builtin_..) - if (IsBuiltinFunction(use->decl(), use->symbol_name()) { + if (is_builtin_function && !is_builtin_function_with_mappings) { VERRS(6) << "Ignoring use of " << use->symbol_name() << " (" << use->PrintableUseLoc() << "): built-in function\n"; use->set_ignore_use(); @@ -1254,7 +1267,8 @@ void ProcessFullUse(OneUse* use, // the language requires). // TODO(csilvers): remove this when we resolve the bugs with macros/typedefs. if (preprocessor_info->FileTransitivelyIncludes( - GetFileEntry(use->decl()), GetFileEntry(use->use_loc()))) { + GetFileEntry(use->decl()), GetFileEntry(use->use_loc())) && + !is_builtin_function_with_mappings) { VERRS(6) << "Ignoring use of " << use->symbol_name() << " (" << use->PrintableUseLoc() << "): 'backwards' #include\n"; use->set_ignore_use(); diff --git a/run_iwyu_tests.py b/run_iwyu_tests.py index 8ed3c22..bd72914 100755 --- a/run_iwyu_tests.py +++ b/run_iwyu_tests.py @@ -63,6 +63,7 @@ class OneIwyuTest(unittest.TestCase): flags_map = { 'backwards_includes.cc': [self.CheckAlsoExtension('-d*.h')], 'badinc.cc': [self.MappingFile('badinc.imp')], + 'built_ins_with_mapping.cc': [self.MappingFile('built_ins_with_mapping.imp')], 'check_also.cc': [self.CheckAlsoExtension('-d1.h')], 'implicit_ctor.cc': [self.CheckAlsoExtension('-d1.h')], 'iwyu_stricter_than_cpp.cc': [self.CheckAlsoExtension('-autocast.h'), @@ -125,6 +126,7 @@ class OneIwyuTest(unittest.TestCase): 'backwards_includes.cc': ['.'], 'badinc.cc': ['.'], 'badinc-extradef.cc': ['.'], + 'built_ins_with_mapping.cc': ['.'], 'funcptrs.cc': ['.'], 'casts.cc': ['.'], 'catch.cc': ['.'], diff --git a/tests/cxx/built_ins_with_mapping-d1.h b/tests/cxx/built_ins_with_mapping-d1.h new file mode 100644 index 0000000..9f76ff1 --- /dev/null +++ b/tests/cxx/built_ins_with_mapping-d1.h @@ -0,0 +1,15 @@ +//===--- built_ins_with_mapping-d1.h - test input file for iwyu -----------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef INCLUDE_WHAT_YOU_USE_TESTS_CXX_BUILT_INS_WITH_MAPPING_D1_H_ +#define INCLUDE_WHAT_YOU_USE_TESTS_CXX_BUILT_INS_WITH_MAPPING_D1_H_ + +int i = __builtin_expect(0, 0); + +#endif // INCLUDE_WHAT_YOU_USE_TESTS_CXX_BUILT_INS_WITH_MAPPING_D1_H_ diff --git a/tests/cxx/built_ins_with_mapping.cc b/tests/cxx/built_ins_with_mapping.cc new file mode 100644 index 0000000..b678cf1 --- /dev/null +++ b/tests/cxx/built_ins_with_mapping.cc @@ -0,0 +1,40 @@ +//===--- built_ins_with_mapping.cc - test input file for iwyu -------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "tests/cxx/built_ins_with_mapping.h" +#include "tests/cxx/built_ins_with_mapping-d1.h" + +// Normally if we use a builtin function IWYU will ignore uses of it. +// However, if there is a mapping defined for that builtin then it should be +// respected. +// Clang considers the definition of a builtin to be at its first use, so we +// have two test cases: + +// First test case for a builtin which was already used in a header we included +// IWYU: __builtin_expect is defined in...*which isn't directly #included. +int j = __builtin_expect(i, 0); +// Second test case for a first use of a builtin +// IWYU: __builtin_strlen is defined in...*which isn't directly #included. +int k = __builtin_strlen(""); + +/**** IWYU_SUMMARY + +tests/cxx/built_ins_with_mapping.cc should add these lines: +#include "tests/cxx/built_ins_with_mapping-d2.h" +#include "tests/cxx/built_ins_with_mapping-d3.h" + +tests/cxx/built_ins_with_mapping.cc should remove these lines: + +The full include-list for tests/cxx/built_ins_with_mapping.cc: +#include "tests/cxx/built_ins_with_mapping.h" +#include "tests/cxx/built_ins_with_mapping-d1.h" // for i +#include "tests/cxx/built_ins_with_mapping-d2.h" // for __builtin_expect +#include "tests/cxx/built_ins_with_mapping-d3.h" // for __builtin_strlen + +***** IWYU_SUMMARY */ diff --git a/tests/cxx/built_ins_with_mapping.h b/tests/cxx/built_ins_with_mapping.h new file mode 100644 index 0000000..bee9eb3 --- /dev/null +++ b/tests/cxx/built_ins_with_mapping.h @@ -0,0 +1,34 @@ +//===--- built_ins_with_mapping.h - test input file for iwyu --------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef INCLUDE_WHAT_YOU_USE_TESTS_CXX_BUILT_INS_WITH_MAPPING_H_ +#define INCLUDE_WHAT_YOU_USE_TESTS_CXX_BUILT_INS_WITH_MAPPING_H_ + +// This is to simulate the situation where a builtin exists on some compilers, +// and not others, so we need a mapping. However, we need to check that the +// header mapped to (this header, in this case) is not forced to include itself +// if it uses that builtin. + +void __builtin_invented_for_test(); + +inline void f() +{ + // A regular function mapped to this file + __builtin_invented_for_test(); + // A builtin mapped to this file + __builtin_strcmp("", ""); +} + +#endif // INCLUDE_WHAT_YOU_USE_TESTS_CXX_BUILT_INS_WITH_MAPPING_H_ + +/**** IWYU_SUMMARY + +(tests/cxx/built_ins_with_mapping.h has correct #includes/fwd-decls) + +***** IWYU_SUMMARY */ diff --git a/tests/cxx/built_ins_with_mapping.imp b/tests/cxx/built_ins_with_mapping.imp new file mode 100644 index 0000000..557c436 --- /dev/null +++ b/tests/cxx/built_ins_with_mapping.imp @@ -0,0 +1,9 @@ +# Header mappings for IWYU tests. +# Note that some headers listed here don't actually exist; we just want +# IWYU to suggest using them +[ + { symbol: ["__builtin_expect", "private", "\"tests/cxx/built_ins_with_mapping-d2.h\"", "public"] }, + { symbol: ["__builtin_strlen", "private", "\"tests/cxx/built_ins_with_mapping-d3.h\"", "public"] }, + { symbol: ["__builtin_strcmp", "private", "\"tests/cxx/built_ins_with_mapping.h\"", "public"] }, + { symbol: ["__builtin_invented_for_test", "private", "\"tests/cxx/built_ins_with_mapping.h\"", "public"] }, +]