github / codeql-coding-standards

This repository contains CodeQL queries and libraries which support various Coding Standards.
MIT License
127 stars 58 forks source link

`A7-1-7`: Avoid reporting the same expression or declaration in multiple template instantiations #383

Closed lcartey closed 4 months ago

lcartey commented 1 year ago

Affected rules

Description

In a template class or template function we may report the same logical expression in multiple template instantiations.

Example

template <typename T>
class Foo {
  void bar(T t) { t; } // COMPLIANT - but erroneously reported
};

void test() {
  Foo<int> i;
  Foo<float> f;
  i.bar(1);
  f.bar(1.0f);
}
rvermeulen commented 8 months ago

Cannot reproduce an amended version of the test case (bar is private). For each instantiation we get an ExprStmt for t in bar, however the location for each of those is the same and the query already excludes different ExprStmts with the same location.

@lcartey Is there more context that we can use to reproduce?

lcartey commented 4 months ago

Re-reviewing the original bug report I believe I misdiagnosed this issue - it is actually completely unrelated to templates.

A fuller reproduction case is as follows:

class Test {
public:
  friend constexpr void swap(Test &lhs, Test &rhs) noexcept { lhs.swap(rhs); }
  void swap(Test &other) noexcept;
};

void test_swap() {
  Test a1, a2;
  swap(a1, a2);
}

The query reports that the function swap is defined on the same line as the expression statement lhs.swap(rhs). Reviewing the AUTOSAR rule, I don't believe the intention is to report such cases - the rule itself does not mention functions either way, and it certainly does not make sense to report the function declaration in which an expression statement is declared as contravening in this case.

As the problem is distinctly different from this initial bug report, I'm closing this as "Doesn't reproduce" and opening a new issue here to track the function problem: https://github.com/github/codeql-coding-standards/issues/628

lcartey commented 4 months ago

In debugging this issue, I identified a couple more problems with A7-1-7 and logged those separately: