[clang-tools-extra] f325b5b - [clang-tidy] Support std::string_view in readability-redundant-string-cstr
Author: Tamas Berghammer
Date: 2023-01-02T06:24:00-05:00
New Revision: f325b5b8cb1d6c16dd04c88380caea7b0a7750eb
URL:
https://github.com/llvm/llvm-project/commit/f325b5b8cb1d6c16dd04c88380caea7b0a7750eb
DIFF:
https://github.com/llvm/llvm-project/commit/f325b5b8cb1d6c16dd04c88380caea7b0a7750eb.diff
LOG: [clang-tidy] Support std::string_view in readability-redundant-string-cstr
Previously we were matching constructor calls for std::string and
llvm::StringRef and this change extends this set with including
std::string_view as well.
Reviewed By: njames93, carlosgalvezp
Differential Revision: https://reviews.llvm.org/D140018
Added:
Modified:
clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
Removed:
diff --git
a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
index 79211731ff7be..9ecd6d612e0c8 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -86,6 +86,11 @@ void RedundantStringCStrCheck::registerMatchers(
// be present explicitly.
hasArgument(1, cxxDefaultArgExpr();
+ // Match string constructor.
+ const auto StringViewConstructorExpr = cxxConstructExpr(
+ argumentCountIs(1),
+ hasDeclaration(cxxMethodDecl(hasName("basic_string_view";
+
// Match a call to the string 'c_str()' method.
const auto StringCStrCallExpr =
cxxMemberCallExpr(on(StringExpr.bind("arg")),
@@ -101,7 +106,8 @@ void RedundantStringCStrCheck::registerMatchers(
traverse(
TK_AsIs,
cxxConstructExpr(
- StringConstructorExpr, hasArgument(0, StringCStrCallExpr),
+ anyOf(StringConstructorExpr, StringViewConstructorExpr),
+ hasArgument(0, StringCStrCallExpr),
unless(anyOf(HasRValueTempParent, hasParent(cxxBindTemporaryExpr(
HasRValueTempParent)),
this);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 2135dd1e01be0..580776b76fd14 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -204,6 +204,10 @@ Changes in existing checks
The check now skips concept definitions since redundant expressions still
make sense
inside them.
+- Support removing ``c_str`` calls from ``std::string_view`` constructor calls
in
+ :doc: `readability-redundant-string-cstr
`
+ check.
+
Removed checks
^^
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
index e1df810b3..ed50ad16f2423 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
@@ -48,6 +48,15 @@ typedef basic_string,
std::allocator> string;
typedef basic_string,
std::allocator> wstring;
typedef basic_string, std::allocator>
u16string;
typedef basic_string, std::allocator>
u32string;
+
+template
+struct basic_string_view {
+ basic_string_view(const C* s);
+};
+typedef basic_string_view> string_view;
+typedef basic_string_view> wstring_view;
+typedef basic_string_view> u16string_view;
+typedef basic_string_view> u32string_view;
}
std::string operator+(const std::string&, const std::string&);
@@ -169,6 +178,15 @@ void f6(const std::string &s) {
tmp.insert(1, s);
tmp.insert(1, s.c_str(), 2);
}
+void f7(std::string_view sv) {
+ std::string s;
+ f7(s.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str'
[readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}f7(s);{{$}}
+ f7(s.data());
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data'
[readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}f7(s);{{$}}
+}
// Tests for std::wstring.
@@ -177,6 +195,15 @@ void g1(const std::wstring &s) {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str'
[readability-redundant-string-cstr]
// CHECK-FIXES: {{^ }}g1(s);{{$}}
}
+void g2(std::wstring_view sv) {
+ std::wstring s;
+ g2(s.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str'
[readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}g2(s);{{$}}
+ g2(s.data());
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data'
[readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}g2(s);{{$}}
+}
// Tests for std::u16string.
@@ -185,6 +212,15 @@ void h1(const std::u
[libcxxabi] r303737 - __cxa_demangle: Fix constructor cv qualifier handling
Author: tberghammer
Date: Wed May 24 06:21:34 2017
New Revision: 303737
URL: http://llvm.org/viewvc/llvm-project?rev=303737&view=rev
Log:
__cxa_demangle: Fix constructor cv qualifier handling
Summary:
Previously if we parsed a constructor then we set parsed_ctor_dtor_cv
to true and never reseted it. This causes issue when a template argument
references a constructor (e.g. type of lambda defined inside a
constructor) as we will have the parsed_ctor_dtor_cv flag set what will
cause issues when parsing later arguments.
Reviewers: EricWF, compnerd
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D33385
Modified:
libcxxabi/trunk/src/cxa_demangle.cpp
libcxxabi/trunk/test/test_demangle.pass.cpp
Modified: libcxxabi/trunk/src/cxa_demangle.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_demangle.cpp?rev=303737&r1=303736&r2=303737&view=diff
==
--- libcxxabi/trunk/src/cxa_demangle.cpp (original)
+++ libcxxabi/trunk/src/cxa_demangle.cpp Wed May 24 06:21:34 2017
@@ -4571,6 +4571,8 @@ parse_encoding(const char* first, const
save_value sb(db.tag_templates);
if (db.encoding_depth > 1)
db.tag_templates = true;
+save_value
sp(db.parsed_ctor_dtor_cv);
+db.parsed_ctor_dtor_cv = false;
switch (*first)
{
case 'G':
Modified: libcxxabi/trunk/test/test_demangle.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/test/test_demangle.pass.cpp?rev=303737&r1=303736&r2=303737&view=diff
==
--- libcxxabi/trunk/test/test_demangle.pass.cpp (original)
+++ libcxxabi/trunk/test/test_demangle.pass.cpp Wed May 24 06:21:34 2017
@@ -29500,6 +29500,7 @@ const char* cases[][2] =
{"_ZZ2f6vE1b", "f6()::b"},
{"_ZNV3$_35test9Ev", "$_3::test9() volatile"},
{"_ZN5Test8I3$_2EC1ES0_", "Test8<$_2>::Test8($_2)"},
+{"_Z3fooIZN3BarC1EvE3$_0EvT_", "void
foo(Bar::Bar()::$_0)"},
{"_ZGVZN1N1gEvE1a", "guard variable for N::g()::a"},
{"_ZplRK1YRA100_P1X", "operator+(Y const&, X* (&) [100])"},
{"_Z1fno", "f(__int128, unsigned __int128)"},
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14354: Add new compiler flag to enable the generation of dwarf accelerator tables
tberghammer abandoned this revision. tberghammer added a comment. Thank you for adding the new debugger tuning feature. I am using LLDB so "-glldb" will work for me perfectly. There is one thing we might consider is that the accelerator tables increasing the size of the binary by ~10% (measured on clang) so I don't know if we want to enable them by default or not on any platform. http://reviews.llvm.org/D14354 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14354: Add new compiler flag to enable the generation of dwarf accelerator tables
tberghammer added a comment. I am not in a hurry with getting this change in, but happy to take a look for enabling the tuning if somebody can explain me what you mean under it (I have almost no llvm/clang development experience) http://reviews.llvm.org/D14354 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14354: Add new compiler flag to enable the generation of dwarf accelerator tables
tberghammer added a comment.
If you plan to do the "tuning" in clang in the near future then we can abandon
this change in favor of that one
Comment at: lib/Driver/Tools.cpp:3880
@@ +3879,3 @@
+ // -gdwarf-accel-tables should turn on -g and enable the genereation of the
+ // dwarf acceleration tables in the backend.
+ if (Args.hasArg(options::OPT_gdwarf_accel_tables)) {
probinson wrote:
> This is not how other similar options work (-ggnu-pubnames, -gdwarf-aranges).
> Do you have a particular reason why this one should imply -g?
No, I am perfectly happy if it don't imply '-g'
http://reviews.llvm.org/D14354
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14354: Add new compiler flag to enable the generation of dwarf accelerator tables
tberghammer added a comment. In http://reviews.llvm.org/D14354#282870, @probinson wrote: > So, currently you get accel tables by default for platforms that "tune" for > LLDB, currently Darwin and FreeBSD. > Are you wanting to use LLDB on another platform, or did you want accel > tables for a different debugger? > > (Eventually I will have a chance to expose the "tuning" up through Clang, and > if you're running LLDB on another platform, that will be the easier way to > get what you want.) We are using LLDB for Android (as part of Android Studio) and some of us also use if for Linux (I hope we can get higher adaptation soon). Comment at: test/Driver/dwarf-accel.c:4 @@ +3,3 @@ +// RUN: %clang -target x86_64-unknown-linux-gnu -gdwarf-accel-tables -c -### %s 2> %t +// RUN: FileCheck -check-prefix=CHECK < %t %s +// probinson wrote: > Normally you'd just pipe the output to FileCheck rather than write a temp > file, because that's more efficient. There are lots of examples to crib from > in other tests. I copied this pattern from split-dwarf.c but will check out how can I do it with piping http://reviews.llvm.org/D14354 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14354: Add new compiler flag to enable the generation of dwarf accelerator tables
Adding the dwarf accelerator tables is increasing the size of libclang.so
(ToT) by ~12% so I don't think we want to generate them by default. Also
these sections are not specified by any dwarf standard at the moment so
emitting them by default would be strange in my opinion.
On Thu, Nov 5, 2015 at 8:17 AM Saleem Abdulrasool
wrote:
>
>
> On Wednesday, November 4, 2015, Tamas Berghammer via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> tberghammer created this revision.
>> tberghammer added a reviewer: echristo.
>> tberghammer added a subscriber: cfe-commits.
>>
>> Add new compiler flag to enable the generation of dwarf accelerator tables
>>
>>
> Is the additional information large enough to require a new flag? They
> wouldn't break split debug even though they don't work there yet, so if the
> impact isn't large enough, might make sense to just enable them on -g.
>
>
>> The dwarf accelerator tables already generated on darwin platforms. This
>> CL ands a new flag to clang to make it possible to enable the generation of
>> these tables on other platforms also.
>>
>> Note: Currently the accelerator table generation code isn't working when
>> split dwarf is enabled for several reasons (accelerator tables aren't
>> copied to dwo file, they contain relocation entries for the .debug_str.dwo
>> sections). These issues should be addressed separately.
>>
>> http://reviews.llvm.org/D14354
>>
>> Files:
>> include/clang/Driver/Options.td
>> lib/Driver/Tools.cpp
>>
>> Index: lib/Driver/Tools.cpp
>> ===
>> --- lib/Driver/Tools.cpp
>> +++ lib/Driver/Tools.cpp
>> @@ -3876,6 +3876,14 @@
>> CmdArgs.push_back("-split-dwarf=Enable");
>>}
>>
>> + // -gdwarf-accel-tables should turn on -g and enable the genereation
>> of the
>> + // dwarf acceleration tables in the backend.
>> + if (Args.hasArg(options::OPT_gdwarf_accel_tables)) {
>> +DebugInfoKind = CodeGenOptions::LimitedDebugInfo;
>> +CmdArgs.push_back("-backend-option");
>> +CmdArgs.push_back("-dwarf-accel-tables=Enable");
>> + }
>> +
>>// After we've dealt with all combinations of things that could
>>// make DebugInfoKind be other than None or DebugLineTablesOnly,
>>// figure out if we need to "upgrade" it to standalone debug info.
>> Index: include/clang/Driver/Options.td
>> ===
>> --- include/clang/Driver/Options.td
>> +++ include/clang/Driver/Options.td
>> @@ -1161,6 +1161,7 @@
>> def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group;
>> def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group;
>> def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group;
>> +def gdwarf_accel_tables : Flag<["-"], "gdwarf-accel-tables">,
>> Group;
>> def gmodules : Flag <["-"], "gmodules">, Group,
>>HelpText<"Generate debug info with external references to clang
>> modules"
>> " or precompiled headers">;
>>
>>
>>
>
> --
> Saleem Abdulrasool
> compnerd (at) compnerd (dot) org
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14354: Add new compiler flag to enable the generation of dwarf accelerator tables
tberghammer updated this revision to Diff 39385.
tberghammer added a comment.
Add a test (I have no experience writing clang tests so please let me know if I
approach is wrong)
http://reviews.llvm.org/D14354
Files:
include/clang/Driver/Options.td
lib/Driver/Tools.cpp
test/Driver/dwarf-accel.c
Index: test/Driver/dwarf-accel.c
===
--- /dev/null
+++ test/Driver/dwarf-accel.c
@@ -0,0 +1,6 @@
+// Check that -gdwarf-accel-tables requests the emission of the accelerator
tables
+//
+// RUN: %clang -target x86_64-unknown-linux-gnu -gdwarf-accel-tables -c -###
%s 2> %t
+// RUN: FileCheck -check-prefix=CHECK < %t %s
+//
+// CHECK: -dwarf-accel-tables=Enable
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3876,6 +3876,14 @@
CmdArgs.push_back("-split-dwarf=Enable");
}
+ // -gdwarf-accel-tables should turn on -g and enable the genereation of the
+ // dwarf acceleration tables in the backend.
+ if (Args.hasArg(options::OPT_gdwarf_accel_tables)) {
+DebugInfoKind = CodeGenOptions::LimitedDebugInfo;
+CmdArgs.push_back("-backend-option");
+CmdArgs.push_back("-dwarf-accel-tables=Enable");
+ }
+
// After we've dealt with all combinations of things that could
// make DebugInfoKind be other than None or DebugLineTablesOnly,
// figure out if we need to "upgrade" it to standalone debug info.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1161,6 +1161,7 @@
def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group;
def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group;
def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group;
+def gdwarf_accel_tables : Flag<["-"], "gdwarf-accel-tables">,
Group;
def gmodules : Flag <["-"], "gmodules">, Group,
HelpText<"Generate debug info with external references to clang modules"
" or precompiled headers">;
Index: test/Driver/dwarf-accel.c
===
--- /dev/null
+++ test/Driver/dwarf-accel.c
@@ -0,0 +1,6 @@
+// Check that -gdwarf-accel-tables requests the emission of the accelerator tables
+//
+// RUN: %clang -target x86_64-unknown-linux-gnu -gdwarf-accel-tables -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK < %t %s
+//
+// CHECK: -dwarf-accel-tables=Enable
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3876,6 +3876,14 @@
CmdArgs.push_back("-split-dwarf=Enable");
}
+ // -gdwarf-accel-tables should turn on -g and enable the genereation of the
+ // dwarf acceleration tables in the backend.
+ if (Args.hasArg(options::OPT_gdwarf_accel_tables)) {
+DebugInfoKind = CodeGenOptions::LimitedDebugInfo;
+CmdArgs.push_back("-backend-option");
+CmdArgs.push_back("-dwarf-accel-tables=Enable");
+ }
+
// After we've dealt with all combinations of things that could
// make DebugInfoKind be other than None or DebugLineTablesOnly,
// figure out if we need to "upgrade" it to standalone debug info.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1161,6 +1161,7 @@
def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group;
def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group;
def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group;
+def gdwarf_accel_tables : Flag<["-"], "gdwarf-accel-tables">, Group;
def gmodules : Flag <["-"], "gmodules">, Group,
HelpText<"Generate debug info with external references to clang modules"
" or precompiled headers">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D14354: Add new compiler flag to enable the generation of dwarf accelerator tables
tberghammer created this revision.
tberghammer added a reviewer: echristo.
tberghammer added a subscriber: cfe-commits.
Add new compiler flag to enable the generation of dwarf accelerator tables
The dwarf accelerator tables already generated on darwin platforms. This CL
ands a new flag to clang to make it possible to enable the generation of these
tables on other platforms also.
Note: Currently the accelerator table generation code isn't working when split
dwarf is enabled for several reasons (accelerator tables aren't copied to dwo
file, they contain relocation entries for the .debug_str.dwo sections). These
issues should be addressed separately.
http://reviews.llvm.org/D14354
Files:
include/clang/Driver/Options.td
lib/Driver/Tools.cpp
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3876,6 +3876,14 @@
CmdArgs.push_back("-split-dwarf=Enable");
}
+ // -gdwarf-accel-tables should turn on -g and enable the genereation of the
+ // dwarf acceleration tables in the backend.
+ if (Args.hasArg(options::OPT_gdwarf_accel_tables)) {
+DebugInfoKind = CodeGenOptions::LimitedDebugInfo;
+CmdArgs.push_back("-backend-option");
+CmdArgs.push_back("-dwarf-accel-tables=Enable");
+ }
+
// After we've dealt with all combinations of things that could
// make DebugInfoKind be other than None or DebugLineTablesOnly,
// figure out if we need to "upgrade" it to standalone debug info.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1161,6 +1161,7 @@
def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group;
def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group;
def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group;
+def gdwarf_accel_tables : Flag<["-"], "gdwarf-accel-tables">,
Group;
def gmodules : Flag <["-"], "gmodules">, Group,
HelpText<"Generate debug info with external references to clang modules"
" or precompiled headers">;
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3876,6 +3876,14 @@
CmdArgs.push_back("-split-dwarf=Enable");
}
+ // -gdwarf-accel-tables should turn on -g and enable the genereation of the
+ // dwarf acceleration tables in the backend.
+ if (Args.hasArg(options::OPT_gdwarf_accel_tables)) {
+DebugInfoKind = CodeGenOptions::LimitedDebugInfo;
+CmdArgs.push_back("-backend-option");
+CmdArgs.push_back("-dwarf-accel-tables=Enable");
+ }
+
// After we've dealt with all combinations of things that could
// make DebugInfoKind be other than None or DebugLineTablesOnly,
// figure out if we need to "upgrade" it to standalone debug info.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1161,6 +1161,7 @@
def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group;
def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group;
def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group;
+def gdwarf_accel_tables : Flag<["-"], "gdwarf-accel-tables">, Group;
def gmodules : Flag <["-"], "gmodules">, Group,
HelpText<"Generate debug info with external references to clang modules"
" or precompiled headers">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
