[clang-tidy] aliases for safety module from section 8 to 12 done

[JonasToth]

Summary

This commit will add aliases into the safety module from other modules.
Since the original purpose of the effort was to implement the High Integrity Coding Standard,
Jonathan (jbcoe) and me had the idea to start out by figuring out what is already done
in clang-tidy.
Splitting the whole standard into parts, this work checked the section 8 to 12.

I made a todolist for me, which can be seen here: https://github.com/JonasToth/HighIntegrityTooling

When we are done aliasing, we create real todos and would like to migrate the list to issues
in the llvm issue tracker.

Please give your opinion on that approach.

[JonasToth]

[Fix] typos in the module file - actually builds now :D

[alexfh]

docs/ReleaseNotes.rst

70–74

Should we have a separate module for the High Integrity C++ Coding Standard as we have for CERT? It seems natural to be able to check the code for HIC++ conformance instead of checking it for conformance with some "safety" checks. Isn't it?

docs/clang-tidy/checks/safety-explicit-conversions.rst

6

How about adding links to the specific HIC++ rule this check enforces? Same for other checks.

[JonasToth]

docs/ReleaseNotes.rst

70–74

Yes, AFAIK was the safety module the starting point of that standard. But the naming is not explicit about it. Maybe we could discuss about a rebranding of the safety module. @jbcoe what do you think?

Furthermore, is it possible to configure aliased checks specially? It makes sense to set the default values/configuration so that it truly confirms.
Most aliases are just "we can do it with this one". Overview pages for different standards would be interesting as well, but out of scope for now, i would say.

docs/clang-tidy/checks/safety-explicit-conversions.rst

6

Yup. I will update when the naming issue from above is clearer.

[aaron.ballman]

docs/ReleaseNotes.rst

70–74

I think it's a good idea -- but, as with CERT, we should make sure we have the permission to use that specific name in our documentation and naming of the module. See LICENSE.TXT and clang-tidy/cert/LICENSE.TXT.

[jbcoe]

docs/ReleaseNotes.rst

70–74

I contacted Programming Research, the authors of HIC++ and have not received a reply.

I would like to name this clang-tidy module after HIC++ but am not happy to do so without permission as it suggests endorsement.

As Aaron suggests, without permission we are probably better off rescinding references to HIC++ entirely and leaving a generic (and still very useful) safety module in place.

[aaron.ballman]

docs/ReleaseNotes.rst

70–74

That wasn't what I was suggesting -- I think having the module would be natural and consistent. The way I read the Conditions of Use is that it cannot imply *PRQA* endorses this use, which I don't think we're in danger of here.

I think we're fine so long as we properly attribute in the license (because we may be using names from the coding standard). Of course, IANAL, etc. ;-)

[JonasToth]

docs/ReleaseNotes.rst

70–74

Programming Research LTD business is to make static code analysis tools.
so i think, we cut their business interest implementing their HIC++ -standard.

probably better to be on the safe site and say the safety module is inspired by HIC++ and others. CERT and MISRA have stuff to say about c++ as well right?
i think we should be sure that we are allowed to use their name and implement their standard, because if we are not, it makes sense for them to sue. at least my thoughts.

maybe we can implement in the old categories + safety and provide example configuration files that would enforce one standard.

@jbcoe did you ask a clang/llvm or urself? Maybe their could be an official request to the company, they probably react to that one.

[aaron.ballman]

docs/ReleaseNotes.rst

70–74

Programming Research LTD business is to make static code analysis tools. so i think, we cut their business interest implementing their HIC++ -standard.
probably better to be on the safe site and say the safety module is inspired by HIC++ and others. CERT and MISRA have stuff to say about c++ as well right?

Correct, hence why we've updated the license for CERT. As for HIC++ , their Conditions of Use are pretty easy to read and interpret. Personally, I don't think we have much to worry about in this regard, so long as we perform the proper attribution they ask for.

i think we should be sure that we are allowed to use their name and implement their standard, because if we are not, it makes sense for them to sue. at least my thoughts.

We have that permission from CERT. We have no such permission from MISRA.

[JonasToth]

docs/ReleaseNotes.rst

70–74

i dont mind these issues tbh :D
just dont want to be the root of later problems :)

iam fine with it, if you are as well.

[jbcoe]

docs/ReleaseNotes.rst

70–74

I asked as myself.

Maybe a senior clang-community member with some CERT background (Aaron?) could ask on behalf of LLVM/Clang.

[aaron.ballman]

docs/ReleaseNotes.rst

70–74

It just so happens there is a PRQA person here at the WG21 meetings. I spoke with him and he said that he will poke the appropriate people to ensure you get a response in a timely manner.

[jbcoe]

docs/ReleaseNotes.rst

70–74

Can we merge this subject to probable revision when PRQA get back to us?
I'm happy to make the code changes needed to rename things when required.

[alexfh]

docs/ReleaseNotes.rst

70–74

If it doesn't take more than a couple of weeks, I'd better wait until we have an answer from PRQA or otherwise reach clarity on how we can refer to the HIC++ standard.

This patch is not going to bitrot soon, and I'd like to avoid unnecessary churn, if possible.

[aaron.ballman]

docs/ReleaseNotes.rst

70–74

I agree. PRQA will hopefully respond soon, or @dberlin will weigh in. Either way, I think the module can wait for a short while to avoid churn.

[jbcoe]

@aaron.ballman any news for PRQA on naming this module? Seems a shame to block good work.

[aaron.ballman]

In D30383#700307, @jbcoe wrote:

@aaron.ballman any news for PRQA on naming this module? Seems a shame to block good work.

I was hoping you had heard from them. Have you received any communications from PRQA (and if so, from who)? I'll see if the person I know from PRQA can get the ball rolling again.

In the meantime, @dberlin, can you comment on whether you think we should be fine to use the name "High Integrity C++ Coding Standard" in our documentation and as a clang-tidy module name (provided we update our license similar to how we did for the CERT module)?

[aaron.ballman]

docs/ReleaseNotes.rst

70–74

I've heard back from PRQA, and they are happy for us to proceed.

"We are perfectly happy for Clang community or any individuals to add in-built or plug-in support for parts of HICPP. We do want an attribution alongside any rule enforcement or rule reference."

I've clarified the attribution with them, and we should proceed in the same way we did with the CERT rules. Basically, we add a LICENSE.TXT to include what's in the Conditions of Use, link to it from the clang-tools-extra LICENSE.TXT, and the documentation for the HIC++ checks should link back to the HIC++ coding standard directly.

I recommend we put this in a module named "hicpp" or "hic++" rather than "safety" to make it clear that the modules relate to the coding standard directly.

[JonasToth]

docs/ReleaseNotes.rst

70–74

That sounds really nice :)
I will proceed with adding the reference to the checks.
I don't know about the License Docs, could someone else do this, or at least point me in the right direction?

Renaming the whole module is ok with me.

[aaron.ballman]

docs/ReleaseNotes.rst

70–74

I think this should be done in two steps (in separate patches). I'll go ahead and take care of the initial patch, and once that's in, you can rebase this patch on top of it. The new patch will:

• Rename the safety directory to hicpp
• Rename the SafetyTidyModule.cpp file to HICPPTidyModule.cpp
• Rename the safety checks to hicpp checks
• Update extra/LICENSE.TXT
• Add extra/clang-tidy/hicpp/LICENSE.TXT

Hopefully this doesn't cause merge issues for you.

[aaron.ballman]

docs/ReleaseNotes.rst

70–74

I've commit this change in r298229, so you should be safe to rebase now.

[JonasToth]

Update the aliases to reflect the new name and link to the related rules in High Integrity C++

• [Merge] Transition to hicpp wording
• [Rename] rename safety files
• Substitute safety with hicpp
• [Doc] references to the section in hicpp added

[JonasToth]

• [Doc] add rule to every mention of the rule number

[jbcoe]

I wonder if it's worth adding a module-level test so that people could easily see that various violations of HICPP are diagnosed?

clang-tools-extra/tests/clang-tidy/test-hicpp.cpp

I'm a bit nervous about code going in without tests and more nervous about the aliases being ok now but wrong later if one of the aliased checks changes its meaning.

[JonasToth]

@jbcoe Yeah that is actually a good point. Can the aliases be configured independent with default arguments? That would be a good improvement as well. For example function-size could take different default parameters.

But i think not every check needs to be tested. Lets say hicpp-use-equals-delete is really simplistic? and the test on the check itself should be sufficient.
Maybe an example configuration file would be nice as well.

[jbcoe]

docs/clang-tidy/checks/hicpp-identifier-naming.rst

7 ↗

(On Diff #92291)

I'm not sure this is the right check.

I think you want readability-named-parameter.
https://clang.llvm.org/extra/clang-tidy/checks/readability-named-parameter.html

readability-identifier-naming takes masses of config and uses it to enforce naming conventions like snake case and trailing _ for member variables.
https://clang.llvm.org/extra/clang-tidy/checks/readability-identifier-naming.html

docs/clang-tidy/checks/hicpp-undelegated-constructor.rst

8 ↗

(On Diff #92291)

typo codeduplication

[JonasToth]

• [Fix] wrong alias mentioned by jonathan

[JonasToth]

docs/clang-tidy/checks/hicpp-identifier-naming.rst

7 ↗

(On Diff #92291)

jup. adjusted.

docs/clang-tidy/checks/hicpp-undelegated-constructor.rst

8 ↗

(On Diff #92291)

what would be the correct spelling? xD
code duplication?

[JonasToth]

• [Fix] typo for code duplication

[aaron.ballman]

clang-tidy/hicpp/HICPPTidyModule.cpp

35–36 ↗

(On Diff #92294)

This should be removed -- it's handled below.

docs/ReleaseNotes.rst

77–78

This should link to the HIC++ standard online.

80

I think we can go ahead and remove this, because *everything* in the hicpp module will be new for this release.

docs/clang-tidy/checks/hicpp-explicit-conversions.rst

7 ↗

(On Diff #92294)

If it doesn't enforce some part of a rule, I think we should call that out explicitly and probably provide wording and an example of what does and does not comply with the rule. Users are going to want to know what level of enforcement there is for a safety-related standard.

Same comments apply to all the docs that claim partial enforcement.

docs/clang-tidy/checks/hicpp-function-size.rst

7 ↗

(On Diff #92294)

Typo: useful

docs/clang-tidy/checks/hicpp-named-parameter.rst

8 ↗

(On Diff #92294)

Drop "the"

docs/clang-tidy/checks/hicpp-new-delete-operators.rst

8 ↗

(On Diff #92294)

Implements <blah> to ensure the new and delete operators have the correct signature.

docs/clang-tidy/checks/hicpp-special-member-functions.rst

7 ↗

(On Diff #92294)

function -> functions

docs/clang-tidy/checks/hicpp-undelegated-constructor.rst

8 ↗

(On Diff #92294)

This last sentence is ungrammatical, but will likely be improved when you add the extra information about what the check does and does not get right versus the rule.

Also, we probably shouldn't say the check "tries" to implement something. It implements partial support. ;-)

docs/clang-tidy/checks/hicpp-use-equals-default.rst

7 ↗

(On Diff #92294)

Implement -> Implements
to default -> to explicitly default

docs/clang-tidy/checks/hicpp-use-equals-delete.rst

8 ↗

(On Diff #92294)

to explicitly default or delete.

docs/clang-tidy/checks/hicpp-use-override.rst

8 ↗

(On Diff #92294)

to declare a virtual function

[JonasToth]

• [Doc] fix the bad grammar and typos

[JonasToth]

fixed the easier issues.

[JonasToth]

• [Doc] grammar and content fixed

[JonasToth]

nontypo fixes.

[JonasToth]

• minor fix, commas not needed in enumeration

[aaron.ballman]

docs/clang-tidy/checks/hicpp-explicit-conversions.rst

9 ↗

(On Diff #93038)

castingchecks -> casting checks

docs/clang-tidy/checks/hicpp-named-parameter.rst

4 ↗

(On Diff #93038)

This underline is too long.

docs/clang-tidy/checks/hicpp-undelegated-constructor.rst

10 ↗

(On Diff #93038)

Rather than saying it's not easily done automatically, it would be more useful to the user to provide examples of what does and does not match the specific coding rule.

[JonasToth]

docs/clang-tidy/checks/hicpp-undelegated-constructor.rst

10 ↗

(On Diff #93038)

I used the examples from the testcase, i hope its ok this way.

[JonasToth]

• [Docs] Typos fixed
• [Doc] improve documentation on what is done in the delegation constructor check

[aaron.ballman]

LGTM!

[jbcoe]

@JonasToth If you don't have commit access let me know and I can merge this patch for you.

[JonasToth]

@jbcoe I dont have them, Thank you for commiting :)

[malcolm.parsons]

The docs for all these aliases are missing http redirects e.g.

.. meta::
   :http-equiv=refresh: 5;URL=modernize-use-equals-delete.html

Without these redirects, the automatically generated list of checks (list.rst) does not say that the checks redirect.

clang-tidy/safety/SafetyTidyModule.cpp

 //===------- SafetyTidyModule.cpp - clang-tidy ----------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "../cppcoreguidelines/ProTypeMemberInitCheck.h"
+#include "../cppcoreguidelines/SpecialMemberFunctionsCheck.h"
+#include "../google/DefaultArgumentsCheck.h"
+#include "../google/ExplicitConstructorCheck.h"
+#include "../misc/NewDeleteOverloadsCheck.h"
+#include "../misc/NoexceptMoveConstructorCheck.h"
+#include "../misc/UndelegatedConstructor.h"
+#include "../misc/UseAfterMoveCheck.h"
+#include "../modernize/UseEqualsDefaultCheck.h"
+#include "../modernize/UseEqualsDeleteCheck.h"
+#include "../modernize/UseOverrideCheck.h"
+#include "../readability/FunctionSizeCheck.h"
+#include "../readability/IdentifierNamingCheck.h"
 #include "NoAssemblerCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace safety {
 
 class SafetyModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
-    CheckFactories.registerCheck<NoAssemblerCheck>(
-        "safety-no-assembler");
+    CheckFactories.registerCheck<google::ExplicitConstructorCheck>(
+        "safety-explicit-conversions");
+    CheckFactories.registerCheck<readability::FunctionSizeCheck>(
+        "safety-function-size");
+    CheckFactories.registerCheck<readability::IdentifierNamingCheck>(
+        "safety-identifier-naming");
+    CheckFactories.registerCheck<misc::UseAfterMoveCheck>(
+        "safety-invalid-access-moved");
+    CheckFactories.registerCheck<cppgoreguidelines::ProTypeMemberInitCheck>(
+        "safety-member-init");
+    CheckFactories.registerCheck<misc::NewDeleteOverloadsCheck>(
+        "safety-new-delete-operators");
+    CheckFactories.registerCheck<misc::NoexceptMoveConstructorCheck>(
+        "safety-noexcept-move");
+    CheckFactories.registerCheck<NoAssemblerCheck>("safety-no-assembler");
+    CheckFactories
+        .registerCheck<cppgoreguidelines::SpecialMemberFunctionsCheck>(
+            "safety-special-member-functions");
+    CheckFactories.registerCheck<misc::UndelegatedConstructor>(
+        "safety-undelegated-constructor");
+    CheckFactories.registerCheck<modernize::UseEqualsDefaultCheck>(
+        "safety-use-equals-default");
+    CheckFactories.registerCheck<modernize::UseEqualsDeleteCheck>(
+        "safety-use-equals-delete");
+    CheckFactories.registerCheck<modernize::UseOverrideCheck>(
+        "safety-use-override");
   }
 };
 
 // Register the SafetyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<SafetyModule>
     X("safety-module", "Adds safety-critical checks.");
 
 } // namespace safety
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the SafetyModule.
 volatile int SafetyModuleAnchorSource = 0;
 
 } // namespace tidy
 } // namespace clang

docs/ReleaseNotes.rst

- New `cert-dcl58-cpp
 
   Finds modification of the ``std`` or ``posix`` namespace.
 
 - New `readability-misleading-indentation
   <http://clang.llvm.org/extra/clang-tidy/checks/readability-misleading-indentation.html>`_ check
 
   Finds misleading indentation where braces should be introduced or the code should be reformatted.
 
+- New `safety` module
+
+  Adds checks that implement the High Integrity C++ Coding Standard and other safety
+  standards. Many checks are aliased to other modules.
+
 - New `safety-no-assembler
   <http://clang.llvm.org/extra/clang-tidy/checks/safety-no-assembler.html>`_ check
 
   Finds uses of inline assembler.
 
 - New `modernize-return-braced-init-list
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-return-braced-init-list.html>`_ check
 
   Finds and replaces explicit calls to the constructor in a return statement by
   a braced initializer list so that the return type is not needlessly repeated.
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 
 Improvements to modularize
 --------------------------
 
 The improvements are...

docs/clang-tidy/checks/safety-explicit-conversions.rst

+.. title:: clang-tidy - safety-explicit-conversions
+
+safety-explicit-conversions
+===========================
+
+This check is an alias for `google-explicit-constructor <google-explicit-constructor.hml>`_.

docs/clang-tidy/checks/safety-function-size.rst

+.. title:: clang-tidy - safety-function-size
+
+safety-function-size
+====================
+
+This check is an alias for `readability-function-size <readability-function-size.hml>`_.

docs/clang-tidy/checks/safety-identifier-naming.rst

+.. title:: clang-tidy - safety-identifier-naming
+
+safety-identifier-naming
+========================
+
+This check is an alias for `readability-identifier-naming <readability-identifier-naming.hml>`_.

docs/clang-tidy/checks/safety-invalid-access-moved.rst

+.. title:: clang-tidy - safety-invalid-access-moved
+
+safety-invalid-access-moved
+===========================
+
+This check is an alias for `misc-use-after-move <misc-use-after-move.hml>`_.

docs/clang-tidy/checks/safety-member-init.rst

+.. title:: clang-tidy - safety-member-init
+
+safety-member-init
+==================
+
+This check is an alias for `cppcoreguidelines-pro-type-member-init <cppcoreguidelines-pro-type-member-init.hml>`_.

docs/clang-tidy/checks/safety-new-delete-operators.rst

+.. title:: clang-tidy - safety-new-delete-operators
+
+safety-new-delete-operators
+===========================
+
+This check is an alias for `misc-new-delete-overloads <misc-new-delete-overloads.hml>`_.

docs/clang-tidy/checks/safety-noexcept-move.rst

+.. title:: clang-tidy - safety-noexcept-move
+
+safety-noexcept-move
+====================
+
+This check is an alias for `misc-noexcept-moveconstructor <misc-noexcept-moveconstructor.hml>`_.

docs/clang-tidy/checks/safety-special-member-functions.rst

+.. title:: clang-tidy - safety-special-member-functions
+
+safety-special-member-functions
+===============================
+
+This check is an alias for `cppcoreguidelines-special-member-functions <cppcoreguidelines-special-member-functions.hml>`_.

docs/clang-tidy/checks/safety-undelegated-constructor.rst

+.. title:: clang-tidy - safety-undelegated-construtor
+
+safety-undelegated-constructor
+==============================
+
+This check is an alias for `misc-undelegated-constructor <misc-undelegated-constructor.hml>`_.

docs/clang-tidy/checks/safety-use-equals-default.rst

+.. title:: clang-tidy - safety-use-equals-defaults
+
+safety-use-equals-default
+=========================
+
+This check is an alias for `modernize-use-equals-default <modernize-use-equals-default.hml>`_.

docs/clang-tidy/checks/safety-use-equals-delete.rst

+.. title:: clang-tidy - safety-use-equals-delete
+
+safety-use-equals-delete
+========================
+
+This check is an alias for `modernize-use-equals-delete <modernize-use-equals-delete.hml>`_.

docs/clang-tidy/checks/safety-use-override.rst

+.. title:: clang-tidy - safety-use-override
+
+safety-use-override
+===================
+
+This check is an alias for `modernize-use-override <modernize-use-override.hml>`_.