[PATCH] D148697: [clang-tidy] Handle more cases of functions which should always be noexcept

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Few issues, first we should have single check for a single cppcoreguidelines rule.
And due to that I'm not fan of merging these things, because for example If I would enable this check in my project I woudn't like to enforce noexcept default constructor or destructor. Simply there are other checks for destructors .

This is why better would be to rename this check into performance-noexcept-move-special-functions, and add other checks for swap, hash, default constructor.
Where one for default constructor could be cpp-core-guidelines only check, and swap, hash could have some performance code.
For example in libcxx if hash function is noexcept, then hash value is not cached in container, but when is not noexcept then it's cached. But thats only libcxx.
Also things like compare operators for me should be noexcept...

If you want to keep single check, I'm fine with it, but consider adding option to disable some parts of it (mainly default constructors), and consider including (not necessary in this patch, all compare operators).

Give it a thing...



================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:347
 
+- Improved :doc:`performance-noexcept-special-functions
+  <clang-tidy/checks/performance/noexcept-special-functions>` to also handle
----------------
Add release notes info about check rename, best as separate entry.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/noexcept-special-functions.rst:10
+`performance-noexcept-special-functions <../performance/noexcept-special-functions.html>`_
+for more information.
----------------
Add link to implemented cpp core guidelines rules.
Add reference 
C.66: Make move operations noexcept
C.83: For value-like types, consider providing a noexcept swap function.
Same goes to operator == (C.86 rule)
And hash C.89 rule



================
Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:195
    `cppcoreguidelines-no-malloc <cppcoreguidelines/no-malloc.html>`_,
+   `cppcoreguidelines-noexcept-special-functions <cppcoreguidelines/noexcept-special-functions.html>`_, "Yes"
    `cppcoreguidelines-owning-memory <cppcoreguidelines/owning-memory.html>`_,
----------------
This list os for checks, 
Add alias under .. csv-table:: Aliases..


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/performance/noexcept-special-functions.rst:7
 
-The check flags user-defined move constructors and assignment operators not
+The check flags user-defined default constructors, move constructors,
+assignment operators, destructors and swap functions not
----------------
user defined default constructor does not need to be noexcept.
There is rule `C.44: Prefer default constructors to be simple and non-throwing` but that.s just a guideline.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/performance/noexcept-special-functions.rst:8
+The check flags user-defined default constructors, move constructors,
+assignment operators, destructors and swap functions not
 marked with ``noexcept`` or marked with ``noexcept(expr)`` where ``expr``
----------------
add entry here why destructors and swap functions should be also noexcept.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148697/new/

https://reviews.llvm.org/D148697