[all-commits] [llvm/llvm-project] 41ee54: Revert "[clang-tidy] Fix readability-redundant-str...

mitchell-stellar via All-commits all-commits at lists.llvm.org
Tue Nov 19 04:54:09 PST 2019


  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 41ee54e5d18858c56725485ef637ad5a686368f4
      https://github.com/llvm/llvm-project/commit/41ee54e5d18858c56725485ef637ad5a686368f4
  Author: Mitchell Balan <mitchell at stellarscience.com>
  Date:   2019-11-19 (Tue, 19 Nov 2019)

  Changed paths:
    M clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
    M clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
    M clang-tools-extra/docs/ReleaseNotes.rst
    M clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst

  Log Message:
  -----------
  Revert "[clang-tidy] Fix readability-redundant-string-init for c++17/c++2a"

This reverts commit 06f3dabe4a2e85a32ade27c0769b6084c828a206.


  Commit: f8901aff4a8f1809d62e0d676a1035099c8f734a
      https://github.com/llvm/llvm-project/commit/f8901aff4a8f1809d62e0d676a1035099c8f734a
  Author: Mitchell Balan <mitchell at stellarscience.com>
  Date:   2019-11-19 (Tue, 19 Nov 2019)

  Changed paths:
    M clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
    M clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
    M clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp

  Log Message:
  -----------
  Revert "[clang-tidy] modernize-use-override new option AllowOverrideAndFinal"

This reverts commit 50e99563fb0459f5160572eef3c4e6062b8ad3f2.


  Commit: 1315f4e009b097d7d1a2a5cf116c1ad55ed5c190
      https://github.com/llvm/llvm-project/commit/1315f4e009b097d7d1a2a5cf116c1ad55ed5c190
  Author: Mitchell Balan <mitchell at stellarscience.com>
  Date:   2019-11-19 (Tue, 19 Nov 2019)

  Changed paths:
    M clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
    M clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
    M clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp

  Log Message:
  -----------
  [clang-tidy] Fix readability-redundant-string-init for c++17/c++2a

Summary:
`readability-redundant-string-init` was one of several clang-tidy checks documented as failing for C++17. (The failure mode in C++17 is that it changes `std::string Name = ""`; to `std::string Name = Name;`, which actually compiles but crashes at run-time.)

Analyzing the AST with `clang -Xclang -ast-dump` showed that the outer `CXXConstructExprs` that previously held the correct SourceRange were being elided in C++17/2a, but the containing `VarDecl` expressions still had all the relevant information. So this patch changes the fix to get its source ranges from `VarDecl`.

It adds one test `std::string g = "u", h = "", i = "uuu", j = "", k;` to confirm proper warnings and fixit replacements in a single `DeclStmt` where some strings require replacement and others don't. The readability-redundant-string-init.cpp and readability-redundant-string-init-msvc.cpp tests now pass for C++11/14/17/2a.

Reviewers: gribozavr, etienneb, alexfh, hokein, aaron.ballman, gribozavr2

Patch by: poelmanc

Subscribers: NoQ, MyDeveloperDay, Eugene.Zelenko, dylanmckay, cfe-commits

Tags: #clang, #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D69238


  Commit: df11117086fe8effdc5abe6a9955f659876dc64f
      https://github.com/llvm/llvm-project/commit/df11117086fe8effdc5abe6a9955f659876dc64f
  Author: Mitchell Balan <mitchell at stellarscience.com>
  Date:   2019-11-19 (Tue, 19 Nov 2019)

  Changed paths:
    M clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
    M clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
    M clang-tools-extra/docs/ReleaseNotes.rst
    M clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
    A clang-tools-extra/test/clang-tidy/checkers/modernize-use-override-allow-override-and-final.cpp

  Log Message:
  -----------
  [clang-tidy] modernize-use-override new option AllowOverrideAndFinal

Summary:
In addition to adding `override` wherever possible, clang-tidy's `modernize-use-override` nicely removes `virtual` when `override` or `final` is specified, and further removes override when final is specified. While this is great default behavior, when code needs to be compiled with gcc at high warning levels that include `gcc -Wsuggest-override` or `gcc -Werror=suggest-override`, clang-tidy's removal of the redundant `override` keyword causes gcc to emit a warning or error. This discrepancy / conflict has been noted by others including a comment on Stack Overflow and by Mozilla's Firefox developers.

This patch adds an AllowOverrideAndFinal option defaulting to 0 - thus preserving current behavior - that when enabled allows both `override` and `final` to co-exist, while still fixing all other issues.

The patch includes a test file verifying all combinations of virtual/override/final, and mentions the new option in the release notes.

Reviewers: alexfh, djasper, JonasToth

Patch by: poelmanc

Subscribers: JonasToth, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D70165


Compare: https://github.com/llvm/llvm-project/compare/6baec971271b...df11117086fe


More information about the All-commits mailing list