[clang-tidy] Add AUTOSAR module

[carlosgalvezp]

Summary

To allow checking the AUTOSAR C++14 Coding Guidelines.

Also an easy check, alias of the C++ Core Guidelines one.

Test and documentation included.

[carlosgalvezp]

Specify C++14 in the module description.

[carlosgalvezp]

Fix ordering in check list.

[carlosgalvezp]

Update release notes.

[whisperity]

"C++14 Guidelines"? So it's always and definitely C++14 specific? What is the licencing approach of this guideline? Looking up with searchers seems to turn up a few PDFs which say --- AUTOSAR CONFIDENTIAL ---, yet they are up and out on the official-looking website.

clang-tools-extra/clang-tidy/autosar/CMakeLists.txt

27

I'm not sure this is needed here, for this module, as of now.

[carlosgalvezp]

"C++14 Guidelines"? So it's always and definitely C++14 specific?

To my knowledge, AUTOSAR handed over the work to MISRA and the current direction is that MISRA will merge AUTOSAR C++14 and older MISRA revisions into a brand-new MISRA release, coming up 202x (The "x" is very unclear here, it could take years to get it in place). So I would be surprised if AUTOSAR released a new version after this. The way they are written they are targeting the use of C++14.

I suppose we can change the description in the future + add configuration to select whether to choose C++14 or something else if that ever comes up, but I think it's a bit premature at this point. Would you agree?

What is the licencing approach of this guideline? Looking up with searchers seems to turn up a few PDFs which say --- AUTOSAR CONFIDENTIAL ---, yet they are up and out on the official-looking website.

Yes, that's strange. The disclaimer says the following:

The material contained in this work is protected by copyright and other types of
intellectual property rights. The commercial exploitation of the material contained in
this work requires a license to such intellectual property rights.
This work may be utilized or reproduced without any modification, in any form or by
any means, for informational purposes only. For any other purpose, no part of the
work may be utilized or reproduced, in any form or by any means, without permission
in writing from the publisher.

I'm not a lawyer but I think we can be sure that this is not a commercial use case? I guess if we were a company selling an AUTOSAR C++14-compliant static analyzer then we would need a license.

[carlosgalvezp]

Removed unnecessary dependency in CMakeLists.txt

[carlosgalvezp]

Also, I've sent a mail to AUTOSAR directly asking for consent, so there's no doubt.

[aaron.ballman]

In D112730#3095767, @carlosgalvezp wrote:

Also, I've sent a mail to AUTOSAR directly asking for consent, so there's no doubt.

Thank you for reaching out to them. I am also not a lawyer, but that license worries me because people DO incorporate clang-tidy into their own commercial offerings (downstream), so we could be creating a legal hassle for them. We may need to reach out to the LLVM Foundation for guidance depending on what the AUTOSAR folks come back with (adding @tonic here for awareness as llvm foundation president).

[carlosgalvezp]

people DO incorporate clang-tidy into their own commercial offerings

Gna, that can be a problem. I wonder if in that case it would be possible to add a few lines into the LLVM Exceptions part of the license. If it's too much of a hassle I guess I'll need to drop it and continue on a local fork unfortunately.

It's interesting however that this fork has implemented quite a few AUTOSAR checks keeping the existing LLVM license:
https://github.com/Bareflank/llvm-project

[tstellar]

@carlosgalvezp Did you write this patch or did you get it from someone else?

[carlosgalvezp]

In D112730#3097091, @tstellar wrote:

@carlosgalvezp Did you write this patch or did you get it from someone else?

@tstellar I wrote it myself (following the existing pattern for the other modules in the llvm-project repo).

Just to clarify, I mentioned the above repo to point out about the license, but I don't plan to take code from there.

[tstellar]

@carlosgalvezp The LLVM Foundation Board will conduct a legal reivew of this patch. Would you be able to share any information you have about the license or usage restrictions for the AUTOSAR specification?

[carlosgalvezp]

In D112730#3116281, @tstellar wrote:

@carlosgalvezp The LLVM Foundation Board will conduct a legal reivew of this patch. Would you be able to share any information you have about the license or usage restrictions for the AUTOSAR specification?

@tstellar Thanks for taking the time to review this, very much appreciated. Absolutely, here's what I know:

• The AUTOSAR C++14 guidelines can be obtained freely from here: https://www.autosar.org/fileadmin/user_upload/standards/adaptive/20-11/AUTOSAR_RS_CPP14Guidelines.pdf
  
  Note: this is the latest version (Release 19-03), compared to the one one could find via a Google search (Release 17-03). It's still publicly available without any kind of paywall or registration needed. It can be searched here: https://www.autosar.org/nc/document-search/

• All pages of the pdf contain the footer "AUTOSAR CONFIDENTIAL". I don't understand what that means legally, given that the document is public.

• The second page of the pdf contains a legal disclaimer, that claims the document shall be used "for information purposes only". For commercial usage, written permission from AUTOSAR must be obtained. I think it would be best if you look at the written statements directly, I might have missed important bits here.

• There exists a llvm-project fork that has implemented some of the checks: https://github.com/Bareflank/llvm-project/tree/bsl-tidy/clang-tools-extra/clang-tidy/bsl. They don't explicitly name Autosar anywhere in the code, but it's clear that they implement Autosar checks. In fact some commits refer to the rule number. The license of this fork is kept as the existing Apache 2.0 with LLVM Exceptions. I don't know if this was agreed with Autosar or simply the author didn't take enough consideration about the licensing aspects. I just want to mention that open-source Autosar checks already exist today under that license, whether it's a mistake or not.

• I have sent an email to AUTOSAR requesting their consent to implement open-source checks. Would you like me to CC you and other members of the Board into that mail? Who should I add?

• More restrictive guidelines (the pdf document/license must be purchased), like MISRA, do allow open-source checkers, as long as only the rule number (not the rule text) is displayed.

I believe that's all I know, I hope it helps in reviewing this issue. Let me know if there are more questions or anything is unclear!

[carlosgalvezp]

@tstellar @tonic I have finally received an answer from AUTOSAR. Let me know if you'd like me to CC you the mail chain to see the exact discussion.

The AUTOSAR confidential statement states that these documents are considered under the license of AUTOSAR, thus a commercial usage without being a partner is not allowed.

As you stated correctly, the C++14 guidelines have been handed over to MISRA and will be released there in spring 2022 according to my knowledge.

Since we had other similar requests already, I can state that this usage of the guidelines is uncritical from AUTOSAR’s perspective.

[tstellar]

In D112730#3133822, @carlosgalvezp wrote:

@tstellar @tonic I have finally received an answer from AUTOSAR. Let me know if you'd like me to CC you the mail chain to see the exact discussion.

Can you cc [e-mail] on the mail chain.

[carlosgalvezp]

Btw can't we just have a license disclaimer like it's done for HiCPP?

https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/hicpp/LICENSE.TXT

[carlosgalvezp]

@tstellar I totally missed your fast reply, sorry about that! I have just forwarded the mail chain to the indicated address. Let me know if there's anything else I can do to help.

[carlosgalvezp]

@tstellar Do you have any updates on the topic? What approximate time frame should I expect? Thanks!

[tonic]

Unfortunately our legal counsel has advised that this patch should not be included into LLVM because the exact patent burden is not disclosed. Therefore, we can not accept this patch into LLVM. I am very sorry to have to report this disappointing news.

[carlosgalvezp]

Thanks a lot @tonic for your update and for the work done in the last months ! It's indeed very sad to hear the news. It's a pity that the work will probably be duplicated in many local forks with sub-optimal solutions instead of a centralized, high-quality, peer-reviewed open-source solution.

I'm not very familiar with the terms so I'm not sure I fully understand the reasons why we are advised not to proceed. Could you clarify/elaborate on what it means "the exact patent burden is not disclosed"? I was hoping that the written consent we got from AUTOSAR would be a strong basis to move forward.

[carlosgalvezp]

In D112730#3316646, @carlosgalvezp wrote:

Thanks a lot @tonic for your update and for the work done in the last months ! It's indeed very sad to hear the news. It's a pity that the work will probably be duplicated in many local forks with sub-optimal solutions instead of a centralized, high-quality, peer-reviewed open-source solution.

I'm not very familiar with the terms so I'm not sure I fully understand the reasons why we are advised not to proceed. Could you clarify/elaborate on what it means "the exact patent burden is not disclosed"? I was hoping that the written consent we got from AUTOSAR would be a strong basis to move forward.

In general it would also be good to know if there was any other impediment to move forward. This information would be extremely valuable to try and do better in the future - I'm thinking on the upcoming MISRA release.

Chris Tapp (chair of MISRA) has been involved in similar discussions in the cfe-dev mailing list, so I think it would be beneficial for him to get this feedback and hopefully make the upcoming MISRA release easy to work with from this standpoint (if MISRA is interested in an open-source checker implementation, of course).

Looking forward to your thoughts :)

[lattner]

Hi all, I'm commenting on this based on my personal opinion, I don't speak for the LLVM board or anyone else. I am also not a lawyer :)

This isn’t a clear cut case (as is typical!). LLVM's approach on patents protection revolves primarily around the terms in the Apache 2 license, which is based on the owner of patents contributing code. Beyond that, we can’t/don't proactively do all the research of potential patent infringement of contributed code.

That said, we have historically NOT taken code that is known to infringe on high risk patents. The one that comes to mind is the (now expired) patents on Steensgaard’s unification-based pointer analysis work. We rejected taking related work until those patents expire.

In this case, AUTOSAR/Parasoft looks like a little company, their spec is clearly saying they have IP rights associated with it, and the risks seem very high to me. I think we should ask for a legal statement signed by a director of the company (not an informal email) saying we can use this in LLVM. The risk is just otherwise too high for the vast community of people who use LLVM.

-Chris

[carlosgalvezp]

Understood, thanks a lot for the clarification and for the time taken!

I will then abandon this patch given the high risk involved. I will forward this feedback to MISRA in case there is a possibility to publish the upcoming revision in open-source-friendly terms.

[lattner]

Thank you so much Carlos, I hope they can come to see the value of having first class support for this in LLVM/Clang. I really appreciate you pushing for this,

-Chris

clang-tools-extra/clang-tidy/CMakeLists.txt

clang_target_link_libraries(clangTidy
     PRIVATE
     clangStaticAnalyzerCore
     clangStaticAnalyzerFrontend
   )
 endif()
 
 # Checks.
 # If you add a check, also add it to ClangTidyForceLinker.h in this directory.
-add_subdirectory(android)
 add_subdirectory(abseil)
 add_subdirectory(altera)
+add_subdirectory(android)
+add_subdirectory(autosar)
 add_subdirectory(boost)
 add_subdirectory(bugprone)
 add_subdirectory(cert)
 add_subdirectory(concurrency)
 add_subdirectory(cppcoreguidelines)
 add_subdirectory(darwin)
 add_subdirectory(fuchsia)
 add_subdirectory(google)
 add_subdirectory(hicpp)
 add_subdirectory(linuxkernel)
 add_subdirectory(llvm)
 add_subdirectory(llvmlibc)
 add_subdirectory(misc)
 add_subdirectory(modernize)
 if(CLANG_TIDY_ENABLE_STATIC_ANALYZER)
   add_subdirectory(mpi)
 endif()
 add_subdirectory(objc)
 add_subdirectory(openmp)
 add_subdirectory(performance)
 add_subdirectory(portability)
 add_subdirectory(readability)
 add_subdirectory(zircon)
 set(ALL_CLANG_TIDY_CHECKS
-  clangTidyAndroidModule
   clangTidyAbseilModule
   clangTidyAlteraModule
+  clangTidyAndroidModule
+  clangTidyAutosarModule
   clangTidyBoostModule
   clangTidyBugproneModule
   clangTidyCERTModule
   clangTidyConcurrencyModule
   clangTidyCppCoreGuidelinesModule
   clangTidyDarwinModule
   clangTidyFuchsiaModule
   clangTidyGoogleModule

clang-tools-extra/clang-tidy/ClangTidyForceLinker.h

 static int LLVM_ATTRIBUTE_UNUSED AlteraModuleAnchorDestination =
     AlteraModuleAnchorSource;
 
 // This anchor is used to force the linker to link the AndroidModule.
 extern volatile int AndroidModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED AndroidModuleAnchorDestination =
     AndroidModuleAnchorSource;
 
+// This anchor is used to force the linker to link the AutosarModule.
+extern volatile int AutosarModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED AutosarModuleAnchorDestination =
+    AutosarModuleAnchorSource;
+
 // This anchor is used to force the linker to link the BoostModule.
 extern volatile int BoostModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED BoostModuleAnchorDestination =
     BoostModuleAnchorSource;
 
 // This anchor is used to force the linker to link the BugproneModule.
 extern volatile int BugproneModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED BugproneModuleAnchorDestination =

clang-tools-extra/clang-tidy/autosar/AutosarTidyModule.cpp

+//===--- AutosarTidyModule.cpp - clang-tidy -------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+#include "../cppcoreguidelines/ProTypeReinterpretCastCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace autosar {
+
+class AutosarModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories
+        .registerCheck<cppcoreguidelines::ProTypeReinterpretCastCheck>(
+            "autosar-a5-2-4-reinterpret-cast");
+  }
+};
+
+} // namespace autosar
+
+// Register the AutosarTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<autosar::AutosarModule>
+    X("autosar-module", "Adds AUTOSAR C++14 clang-tidy checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the AutosarModule.
+volatile int AutosarModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/autosar/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
+  Support
+  )
+
+add_clang_library(clangTidyAutosarModule
+  AutosarTidyModule.cpp
+
+  LINK_LIBS
+  clangTidy
+  clangTidyUtils
+  clangTidyCppCoreGuidelinesModule
+
+  DEPENDS
+  omp_gen
+  )
+
+clang_target_link_libraries(clangTidyAutosarModule
+  PRIVATE
+  clangAnalysis
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangTooling
+  )

clang-tools-extra/docs/ReleaseNotes.rst

 Improvements to clang-rename
 ----------------------------
 
 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
+- New module: `autosar`, covering the AUTOSAR C++14 Coding Guidelines.
+
 - Added support for globbing in `NOLINT*` expressions, to simplify suppressing
   multiple warnings in the same line.
 
 - Added support for `NOLINTBEGIN` ... `NOLINTEND` comments to suppress
   Clang-Tidy warnings over multiple lines.
 
 New checks
 ^^^^^^^^^^

 - New :doc:`readability-data-pointer <clang-tidy/checks/readability-data-pointer>` check.
 
   Finds cases where code could use ``data()`` rather than the address of the
   element at index 0 in a container.
 
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
+- New alias :doc:`autosar-a5-4-2-reinterpret-cast
+  <clang-tidy/checks/autosar-a5-4-2-reinterpret-cast>` to
+  :doc:`cppcoreguidelines-pro-type-reinterpret-cast
+  <clang-tidy/checks/cppcoreguidelines-pro-type-reinterpret-cast>` was added.
+
 - New alias :doc:`cert-exp42-c
   <clang-tidy/checks/cert-exp42-c>` to
   :doc:`bugprone-suspicious-memory-comparison
   <clang-tidy/checks/bugprone-suspicious-memory-comparison>` was added.
 
 - New alias :doc:`cert-flp37-c
   <clang-tidy/checks/cert-flp37-c>` to
   :doc:`bugprone-suspicious-memory-comparison

clang-tools-extra/docs/clang-tidy/checks/autosar-a5-4-2-reinterpret-cast.rst

+.. title:: clang-tidy - autosar-a5-4-2-reinterpret-cast
+.. meta::
+   :http-equiv=refresh: 5;URL=cppcoreguidelines-pro-type-reinterpret-cast.html
+
+autosar-a5-4-2-reinterpret-cast
+===============================
+
+The autosar-a5-4-2-reinterpret-cast check is an alias, please see
+`cppcoreguidelines-pro-type-reinterpret-cast <cppcoreguidelines-pro-type-reinterpret-cast.html>`_
+for more information.

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `android-cloexec-inotify-init <android-cloexec-inotify-init.html>`_,
    `android-cloexec-inotify-init1 <android-cloexec-inotify-init1.html>`_,
    `android-cloexec-memfd-create <android-cloexec-memfd-create.html>`_,
    `android-cloexec-open <android-cloexec-open.html>`_,
    `android-cloexec-pipe <android-cloexec-pipe.html>`_, "Yes"
    `android-cloexec-pipe2 <android-cloexec-pipe2.html>`_,
    `android-cloexec-socket <android-cloexec-socket.html>`_,
    `android-comparison-in-temp-failure-retry <android-comparison-in-temp-failure-retry.html>`_,
+   `autosar-a5-4-2-reinterpret-cast <autosar-a5-4-2-reinterpret-cast.html>`_,
    `boost-use-to-string <boost-use-to-string.html>`_, "Yes"
    `bugprone-argument-comment <bugprone-argument-comment.html>`_, "Yes"
    `bugprone-assert-side-effect <bugprone-assert-side-effect.html>`_,
    `bugprone-bad-signal-to-kill-thread <bugprone-bad-signal-to-kill-thread.html>`_,
    `bugprone-bool-pointer-implicit-conversion <bugprone-bool-pointer-implicit-conversion.html>`_, "Yes"
    `bugprone-branch-clone <bugprone-branch-clone.html>`_,
    `bugprone-copy-constructor-init <bugprone-copy-constructor-init.html>`_, "Yes"
    `bugprone-dangling-handle <bugprone-dangling-handle.html>`_,
.. csv-table::
    `readability-uppercase-literal-suffix <readability-uppercase-literal-suffix.html>`_, "Yes"
    `readability-use-anyofallof <readability-use-anyofallof.html>`_,
    `zircon-temporary-objects <zircon-temporary-objects.html>`_,
 
 
 .. csv-table:: Aliases..
    :header: "Name", "Redirect", "Offers fixes"
 
+   `autosar-a5-4-2-reinterpret-cast <autosar-a5-4-2-reinterpret-cast.html>`_, `cppcoreguidelines-pro-type-reinterpret-cast <cppcoreguidelines-pro-type-reinterpret-cast>`_,
    `cert-con36-c <cert-con36-c.html>`_, `bugprone-spuriously-wake-up-functions <bugprone-spuriously-wake-up-functions.html>`_,
    `cert-con54-cpp <cert-con54-cpp.html>`_, `bugprone-spuriously-wake-up-functions <bugprone-spuriously-wake-up-functions.html>`_,
    `cert-dcl03-c <cert-dcl03-c.html>`_, `misc-static-assert <misc-static-assert.html>`_, "Yes"
    `cert-dcl16-c <cert-dcl16-c.html>`_, `readability-uppercase-literal-suffix <readability-uppercase-literal-suffix.html>`_, "Yes"
    `cert-dcl37-c <cert-dcl37-c.html>`_, `bugprone-reserved-identifier <bugprone-reserved-identifier.html>`_, "Yes"
    `cert-dcl51-cpp <cert-dcl51-cpp.html>`_, `bugprone-reserved-identifier <bugprone-reserved-identifier.html>`_, "Yes"
    `cert-dcl54-cpp <cert-dcl54-cpp.html>`_, `misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
    `cert-dcl59-cpp <cert-dcl59-cpp.html>`_, `google-build-namespaces <google-build-namespaces.html>`_,

clang-tools-extra/docs/clang-tidy/index.rst

 There are currently the following groups of checks:
 
 ====================== =========================================================
 Name prefix            Description
 ====================== =========================================================
 ``abseil-``            Checks related to Abseil library.
 ``altera-``            Checks related to OpenCL programming for FPGAs.
 ``android-``           Checks related to Android.
+``autosar-``           Checks related to AUTOSAR C++14 Coding Guidelines.
 ``boost-``             Checks related to Boost library.
 ``bugprone-``          Checks that target bugprone code constructs.
 ``cert-``              Checks related to CERT Secure Coding Guidelines.
 ``clang-analyzer-``    Clang Static Analyzer checks.
 ``concurrency-``       Checks related to concurrent programming (including
                        threads, fibers, coroutines, etc.).
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
 ``darwin-``            Checks related to Darwin coding conventions.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-reinterpret-cast.cpp

-// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-reinterpret-cast %t
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-reinterpret-cast,autosar-a5-2-4-reinterpret-cast %t
 
 int i = 0;
 void *j;
 void f() { j = reinterpret_cast<void *>(i); }
-// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use reinterpret_cast [autosar-a5-2-4-reinterpret-cast,cppcoreguidelines-pro-type-reinterpret-cast]