Exclude rvalue references from const in AUTOSAR rule 7-1-1.

[fjatWbyT]

Description

In the updated query for A7-1-1, unmodified objects (including arguments) that aren't declared const are warned about missing const.

I have observed the warning for rvalue references (in particular, for forwarding/universal references of variadic arguments) in utilities similar to std::function, although it isn't trivial to reproduce (i.e. not every universal reference to a variadic argument results in the warning).

This change attempts to avoid that case of false positives by generally excluding rvalue references from being warned about a missing const.

Change request type

• Release or process automation (GitHub workflows, internal scripts)
• Internal documentation
• External documentation
• Query files (.ql, .qll, .qls or unit tests)
• External scripts (analysis report or other code shipped as part of a release)

Rules with added or modified queries

• Queries have been modified for the following rules:
  • A7-1-1

Release change checklist

A change note (development_handbook.md#change-notes) is required for any pull request which modifies:

• The structure or layout of the release artifacts.
• The evaluation performance (memory, execution time) of an existing query.
• The results of an existing query in any circumstance.

If you are only adding new rule queries, a change note is not required.

Author: Is a change note required?

• Yes
• No

🚨🚨🚨
Reviewer: Confirm that format of shared queries (not the .qll file, the
.ql file that imports it) is valid by running them within VS Code.

• Confirmed

Reviewer: Confirm that either a change note is not required or the change note is required and has been added.

• Confirmed

Query development review checklist

For PRs that add new queries or modify existing queries, the following checklist should be completed by both the author and reviewer:

Author

• Have all the relevant rule package description files been checked in?
• Have you verified that the metadata properties of each new query is set appropriately?
• Do all the unit tests contain both "COMPLIANT" and "NON_COMPLIANT" cases?
• Are the alert messages properly formatted and consistent with the style guide?
• Have you run the queries on OpenPilot and verified that the performance and results are acceptable?
  As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.
• Does the query have an appropriate level of in-query comments/documentation?
• Have you considered/identified possible edge cases?
• Does the query not reinvent features in the standard library?
• Can the query be simplified further (not golfed!)

Reviewer

• Have all the relevant rule package description files been checked in?
• Have you verified that the metadata properties of each new query is set appropriately?
• Do all the unit tests contain both "COMPLIANT" and "NON_COMPLIANT" cases?
• Are the alert messages properly formatted and consistent with the style guide?
• Have you run the queries on OpenPilot and verified that the performance and results are acceptable?
  As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.
• Does the query have an appropriate level of in-query comments/documentation?
• Have you considered/identified possible edge cases?
• Does the query not reinvent features in the standard library?
• Can the query be simplified further (not golfed!)

[lcartey]

@fjatWbyT How about this test case?

#include <utility>

void variadic_forwarding() {}

template <typename T, typename... Args>
void variadic_forwarding(T &&first, Args &&...rest) {
  first;
  variadic_forwarding(std::forward<Args>(rest)...);
}

int test_variadic_forwarding() { variadic_forwarding(1, 1.1, "a"); }

[fjatWbyT]

Wow, nice one, @lcartey, thanks a lot for looking into it!

With that test case I have indeed been able to reproduce the alert at the rest parameter.

This is the snippet, composed of two independent test cases, I had used:

#include <future>
#include <utility>

template<typename F, typename... Args>
constexpr decltype(auto) f(F&& func, Args&&... args)
{
  return std::forward<F>(func)(std::forward<Args>(args)...);
}

class awaitable
{
  awaitable(std::future<void>&& _future) : future(std::move(_future)) {}

 private:
  std::future<void> future;
};

int main()
{
}

But didn't get alert for any of func, args, or _future. I am not sure at the moment what is the culprit, but it should now be more feasible to find having the new test case. Thanks again!

I will be updating the pull request shortly adding the test case.

[fjatWbyT]

But didn't get alert for any of func, args, or _future. I am not sure at the moment what is the culprit, but it should now be more feasible to find having the new test case. Thanks again!

Ah, it may have to do with actually instantiating the template. So not for the case with _future, but for the other case getting it at args making an actual call to template f.

[fjatWbyT]

I verified the test failed without the change in the previous commit due to

+| test.cpp:93:47:93:50 | rest | Non-constant variable rest points to an object and is not modified. |

[fjatWbyT]

I added all at the end of the file to minimize updates to the expected files due to line numbers. But not sure if due to (auto) formatting rules or else it would be preferable otherwise.