Page MenuHomePhabricator

[clang-tidy] Include std::basic_string_view in readability-redundant-string-init.
ClosedPublic

Authored by ckennelly on Nov 7 2020, 11:33 AM.

Details

Summary

std::string_view("") produces a string_view instance that compares
equal to std::string_view(), but requires more complex initialization
(storing the address of the string literal, rather than zeroing).

Diff Detail

Unit TestsFailed

TimeTest
670 mslinux > LLVM.Other::new-pass-manager.ll
Script: -- : 'RUN: at line 8'; /mnt/disks/ssd0/agent/llvm-project/build/bin/opt -disable-output -disable-verify -debug-pass-manager -passes=no-op-module /mnt/disks/ssd0/agent/llvm-project/llvm/test/Other/new-pass-manager.ll 2>&1 | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/llvm/test/Other/new-pass-manager.ll --check-prefix=CHECK-MODULE-PASS
2,040 mswindows > LLVM.Other::new-pass-manager.ll
Script: -- : 'RUN: at line 8'; c:\ws\w1\llvm-project\premerge-checks\build\bin\opt.exe -disable-output -disable-verify -debug-pass-manager -passes=no-op-module C:\ws\w1\llvm-project\premerge-checks\llvm\test\Other\new-pass-manager.ll 2>&1 | c:\ws\w1\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w1\llvm-project\premerge-checks\llvm\test\Other\new-pass-manager.ll --check-prefix=CHECK-MODULE-PASS

Event Timeline

ckennelly created this revision.Nov 7 2020, 11:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 7 2020, 11:33 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
ckennelly requested review of this revision.Nov 7 2020, 11:33 AM
Eugene.Zelenko added a project: Restricted Project.

You don't really need to duplicate every single std::string test. Just enough so that it detects it, kind of like ::out::TestString. Also can you update the docs and release notes to explain this behaviour.

lebedev.ri retitled this revision from Include std::basic_string_view in readability-redundant-string-init. to [clang-tidy] Include std::basic_string_view in readability-redundant-string-init..Nov 8 2020, 12:12 PM

You don't really need to duplicate every single std::string test. Just enough so that it detects it, kind of like ::out::TestString. Also can you update the docs and release notes to explain this behaviour.

Done

ckennelly updated this revision to Diff 304034.Nov 9 2020, 8:29 PM

Applied feedback from review

  • reduced test cases to verify functionality
  • updated release notes
  • updated docs
aaron.ballman added inline comments.Nov 19 2020, 6:35 AM
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
25

nit: using size_type = decltype(sizeof(0));

30

This constructor doesn't exist according to the standard: http://eel.is/c++draft/string.view.template.general

65

Can you also add tests for wstring_view?

Also, perhaps one for std::string_view foo{}; (to remove the spurious {})

ckennelly updated this revision to Diff 306591.Nov 19 2020, 8:22 PM
ckennelly marked 3 inline comments as done.

Applied review feedback

clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
65

Done for wstring_view

re std::string_view foo{}, it looks like we don't even match std::string{}, https://godbolt.org/z/zaafrb

This revision is now accepted and ready to land.Nov 20 2020, 6:07 AM
This revision was landed with ongoing or failed builds.Nov 20 2020, 7:06 AM
This revision was automatically updated to reflect the committed changes.