[clang-tools-extra] r318840 - [FindAllSymbols] Cache regexes, creating them is expensive
Krzysztof Parzyszek via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 23 09:36:00 PST 2017
Hi,
This broke build on FreeBSD 11:
[100%] Building CXX object
tools/clang/tools/extra/include-fixer/find-all-symbols/CMakeFiles/findAllSymbols.dir/HeaderMapCollector.cpp.o
cd /w/bld/org/tools/clang/tools/extra/include-fixer/find-all-symbols &&
/w/c/clang+llvm-5.0.0-x86_64-unknown-freebsd11/bin/clang++
-DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS
-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
-I/w/bld/org/tools/clang/tools/extra/include-fixer/find-all-symbols
-I/w/src/llvm.org/tools/clang/tools/extra/include-fixer/find-all-symbols
-I/w/src/llvm.org/tools/clang/include -I/w/bld/org/tools/clang/include
-I/w/bld/org/include -I/w/src/llvm.org/include -isystem
/usr/local/include -stdlib=libc++ -fPIC -fvisibility-inlines-hidden
-Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -W
-Wno-unused-parameter -Wwrite-strings -Wcast-qual
-Wmissing-field-initializers -pedantic -Wno-long-long
-Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor
-Wno-comment -Wstring-conversion -ffunction-sections -fdata-sections
-fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -UNDEBUG
-fno-exceptions -fno-rtti -o
CMakeFiles/findAllSymbols.dir/HeaderMapCollector.cpp.o -c
/w/src/llvm.org/tools/clang/tools/extra/include-fixer/find-all-symbols/HeaderMapCollector.cpp
In file included from
/w/src/llvm.org/tools/clang/tools/extra/include-fixer/find-all-symbols/HeaderMapCollector.cpp:10:
In file included from
/w/src/llvm.org/tools/clang/tools/extra/include-fixer/find-all-symbols/HeaderMapCollector.h:13:
In file included from /w/src/llvm.org/include/llvm/ADT/StringMap.h:17:
In file included from /w/src/llvm.org/include/llvm/ADT/StringRef.h:13:
In file included from /w/src/llvm.org/include/llvm/ADT/STLExtras.h:20:
In file included from /w/src/llvm.org/include/llvm/ADT/Optional.h:22:
In file included from /w/src/llvm.org/include/llvm/Support/type_traits.h:19:
/usr/include/c++/v1/utility:315:11: error: call to deleted constructor
of 'llvm::Regex'
: first(__p.first),
^ ~~~~~~~~~
/usr/include/c++/v1/memory:1747:31: note: in instantiation of member
function 'std::__1::pair<llvm::Regex, const char *>::pair' requested here
::new((void*)__p) _Up(_VSTD::forward<_Args>(__args)...);
^
/usr/include/c++/v1/memory:1658:18: note: in instantiation of function
template specialization 'std::__1::allocator<std::__1::pair<llvm::Regex,
const char *> >::construct<std::__1::pair<llvm::Regex, const char *>,
const std::__1::pair<llvm::Regex, const char *> &>' requested here
{__a.construct(__p, _VSTD::forward<_Args>(__args)...);}
^
/usr/include/c++/v1/memory:1504:14: note: in instantiation of function
template specialization
'std::__1::allocator_traits<std::__1::allocator<std::__1::pair<llvm::Regex,
const char *> > >::__construct<std::__1::pair<llvm::Regex, const char
*>, const std::__1::pair<llvm::Regex, const char *> &>' requested here
{__construct(__has_construct<allocator_type, _Tp*, _Args...>(),
^
/usr/include/c++/v1/memory:1620:17: note: in instantiation of function
template specialization
'std::__1::allocator_traits<std::__1::allocator<std::__1::pair<llvm::Regex,
const char *> > >::construct<std::__1::pair<llvm::Regex, const char *>,
const std::__1::pair<llvm::Regex, const char *> &>' requested here
construct(__a, _VSTD::__to_raw_pointer(__end2-1),
_VSTD::move_if_noexcept(*--__end1));
^
/usr/include/c++/v1/vector:892:21: note: in instantiation of function
template specialization
'std::__1::allocator_traits<std::__1::allocator<std::__1::pair<llvm::Regex,
const char *> > >::__construct_backward<std::__1::pair<llvm::Regex,
const char *> *>' requested here
__alloc_traits::__construct_backward(this->__alloc(),
this->__begin_, this->__end_, __v.__begin_);
^
/usr/include/c++/v1/vector:1537:9: note: in instantiation of member
function 'std::__1::vector<std::__1::pair<llvm::Regex, const char *>,
std::__1::allocator<std::__1::pair<llvm::Regex, const char *> >
>::__swap_out_circular_buffer' requested here
__swap_out_circular_buffer(__v);
^
/w/src/llvm.org/tools/clang/tools/extra/include-fixer/find-all-symbols/HeaderMapCollector.cpp:19:33:
note: in instantiation of member function
'std::__1::vector<std::__1::pair<llvm::Regex, const char *>,
std::__1::allocator<std::__1::pair<llvm::Regex, const char *> >
>::reserve' requested here
this->RegexHeaderMappingTable.reserve(RegexHeaderMappingTable->size());
^
/w/src/llvm.org/include/llvm/Support/Regex.h:49:5: note: 'Regex' has
been explicitly marked deleted here
Regex(const Regex &) = delete;
^
1 error generated.
*** Error code 1
-Krzysztof
On 11/22/2017 9:38 AM, Benjamin Kramer via cfe-commits wrote:
> Author: d0k
> Date: Wed Nov 22 07:38:23 2017
> New Revision: 318840
>
> URL: http://llvm.org/viewvc/llvm-project?rev=318840&view=rev
> Log:
> [FindAllSymbols] Cache regexes, creating them is expensive
>
> This is a bit annoying because LLVM regexes are always mutable to store
> errors. Assert that there are never errors and fix broken hardcoded
> regexes.
>
> Modified:
> clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.cpp
> clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.h
> clang-tools-extra/trunk/include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
>
> Modified: clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.cpp?rev=318840&r1=318839&r2=318840&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.cpp (original)
> +++ clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.cpp Wed Nov 22 07:38:23 2017
> @@ -13,6 +13,16 @@
> namespace clang {
> namespace find_all_symbols {
>
> +HeaderMapCollector::HeaderMapCollector(
> + const RegexHeaderMap *RegexHeaderMappingTable) {
> + assert(RegexHeaderMappingTable);
> + this->RegexHeaderMappingTable.reserve(RegexHeaderMappingTable->size());
> + for (const auto &Entry : *RegexHeaderMappingTable) {
> + this->RegexHeaderMappingTable.emplace_back(llvm::Regex(Entry.first),
> + Entry.second);
> + }
> +}
> +
> llvm::StringRef
> HeaderMapCollector::getMappedHeader(llvm::StringRef Header) const {
> auto Iter = HeaderMappingTable.find(Header);
> @@ -20,11 +30,13 @@ HeaderMapCollector::getMappedHeader(llvm
> return Iter->second;
> // If there is no complete header name mapping for this header, check the
> // regex header mapping.
> - if (RegexHeaderMappingTable) {
> - for (const auto &Entry : *RegexHeaderMappingTable) {
> - if (llvm::Regex(Entry.first).match(Header))
> - return Entry.second;
> - }
> + for (auto &Entry : RegexHeaderMappingTable) {
> +#ifndef NDEBUG
> + std::string Dummy;
> + assert(Entry.first.isValid(Dummy) && "Regex should never be invalid!");
> +#endif
> + if (Entry.first.match(Header))
> + return Entry.second;
> }
> return Header;
> }
>
> Modified: clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.h?rev=318840&r1=318839&r2=318840&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.h (original)
> +++ clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.h Wed Nov 22 07:38:23 2017
> @@ -25,10 +25,8 @@ public:
> typedef llvm::StringMap<std::string> HeaderMap;
> typedef std::vector<std::pair<const char *, const char *>> RegexHeaderMap;
>
> - HeaderMapCollector() : RegexHeaderMappingTable(nullptr) {}
> -
> - explicit HeaderMapCollector(const RegexHeaderMap *RegexHeaderMappingTable)
> - : RegexHeaderMappingTable(RegexHeaderMappingTable) {}
> + HeaderMapCollector() = default;
> + explicit HeaderMapCollector(const RegexHeaderMap *RegexHeaderMappingTable);
>
> void addHeaderMapping(llvm::StringRef OrignalHeaderPath,
> llvm::StringRef MappingHeaderPath) {
> @@ -47,8 +45,10 @@ private:
> HeaderMap HeaderMappingTable;
>
> // A map from header patterns to header names.
> - // This is a reference to a hard-coded map.
> - const RegexHeaderMap *const RegexHeaderMappingTable;
> + // The header names are not owned. This is only threadsafe because the regexes
> + // never fail.
> + mutable std::vector<std::pair<llvm::Regex, const char *>>
> + RegexHeaderMappingTable;
> };
>
> } // namespace find_all_symbols
>
> Modified: clang-tools-extra/trunk/include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp?rev=318840&r1=318839&r2=318840&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp (original)
> +++ clang-tools-extra/trunk/include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp Wed Nov 22 07:38:23 2017
> @@ -314,10 +314,10 @@ const HeaderMapCollector::RegexHeaderMap
> {"include/xlocale.h$", "<cstring>"},
> {"bits/atomic_word.h$", "<memory>"},
> {"bits/basic_file.h$", "<fstream>"},
> - {"bits/c++allocator.h$", "<string>"},
> - {"bits/c++config.h$", "<iosfwd>"},
> - {"bits/c++io.h$", "<ios>"},
> - {"bits/c++locale.h$", "<locale>"},
> + {"bits/c\\+\\+allocator.h$", "<string>"},
> + {"bits/c\\+\\+config.h$", "<iosfwd>"},
> + {"bits/c\\+\\+io.h$", "<ios>"},
> + {"bits/c\\+\\+locale.h$", "<locale>"},
> {"bits/cpu_defines.h$", "<iosfwd>"},
> {"bits/ctype_base.h$", "<locale>"},
> {"bits/cxxabi_tweaks.h$", "<cxxabi.h>"},
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
> ---
> This email has been checked for viruses by AVG.
> http://www.avg.com
>
More information about the cfe-commits
mailing list