aaron.ballman added inline comments.
================
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:59-61
@@ +58,5 @@
+ IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+ Options.get("IncludeStyle", "llvm"))) {
+
+ for (const auto &KeyValue :
+ {std::make_pair("memcpy", "std::copy"), {"memset", "std::fill"}}) {
----------------
alexfh wrote:
> I'm not sure this works on MSVC2013. Could someone try this before submitting?
It does not work in MSVC 2013.
================
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:102
@@ +101,3 @@
+ assert(it != Replacements.end() &&
+ "Replacements list does not match list of registered matcher names");
+ const std::string ReplacedName = it->second;
----------------
alexfh wrote:
> Notes are treated completely differently - each note has to be attached to a
> warning.
>
> Clang-tidy can only deal with two severity levels of diagnostics: "warning"
> and "error", but it's better to let them be controlled by the user via
> `-warnings-as-errors=` command-line option or the `WarningsAsErrors`
> configuration file option.
Drat. I am not keen on this being a warning (let alone an error) because it
really doesn't warn the user against anything bad (the type safety argument is
tenuous at best), and it's arguable whether this actually modernizes code. Do
you think there's sufficient utility to this check to warrant it being
default-enabled as part of the modernize suite? For instance, we have 368
instances of memcpy() and 171 instances of std::copy() in the LLVM code base,
which is an example of a very modern C++ code base. I'm not opposed to the
check, just worried that it will drown out diagnostics that have more impact to
the user.
Repository:
rL LLVM
https://reviews.llvm.org/D22725
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
