Wrong suggestion to use pointer to const

I found this case via SonarQube 9.3 and reproduced it with SonarLint 5.4.0.42421

struct Bar
{
    int *p;
};

inline auto foo1(int *const ptr)
{
    return Bar(ptr);
}

inline auto foo2(int *const ptr)
{
    return Bar{ptr};
}

I originally found this in a code snippet that involves std::make_unique and polymorphism, but I managed to reproduce it here.

It seems like the analyzer doesn’t understand that the pointer cannot be a pointer to const when calling constructors with parentheses. But it somehow understands it when using braces? It seems weird for the analyzer to not understand this important aspect of the type system. A pointer to const is not convertible to a pointer to non-const, so this code would not compile if I added the const as suggested.

The same seems to apply for references.

Hi @torgeir.skogen,
Thank you for reporting the suspected false positive. In this case, I do not consider it to be a false positive, because your code fails to compile:

error: no matching conversion for functional-style cast from ‘int *const’ to ‘Bar’

Given the code is incorrect from a C++ compiler point of view, our analyzer does its best to interpret the intent and report the potential issues once the code is fixed. Surely enough, it often fails to do so. However, it does flag the code with a special issue.

There is a rule with ParsingError as a key. You can enable this rule (through the SonarLint JSON setting or in your SQ instance) to make sure that there is no parsing error in the code where the false positive is happening. For example, in SonarLint for VisualStudio, you can put the following into your settings.json:

{
  "sonarlint.rules": {
    "cpp:ParsingError": {
      "level": "On"
    },
    "c:ParsingError": {
      "level": "On"
    }
  }
}

You can fix foo1 either the way you did in foo2 or by adding a constructor to Bar that would take a non-const pointer. Both fixes also remove the S995 report.

However, you mentioned that you’ve encountered the FP in a more complex piece of code. I encourage you to get back to your original code and check if it contains a ParsingError. If it does not, besides trying to simplify it again and watching to not introduce a ParsingError, you can generate a reproducer file:

  • Add the reproducer option to the scanner configuration:
    sonar.cfamily.reproducer= “Full path to the .cpp file that has or include the file that has the false-positive”
  • Re-run the scanner to generate a file named sonar-cfamily.reproducer in the project folder.
  • Please share this file. If you think this file contains private information you can send it privately.

The code compiles in C++20 mode. Compiler Explorer because you are now allowed to initialize aggregates with parenthesis.

Good call on the parsing errors. I enabled those in the quality profile in the projects I manage.

I think I managed to land this reproducer

struct Base
{};

struct Bar : Base
{
    explicit Bar(int *);
};

inline std::unique_ptr<Base> factory(int *const p)
{
    return std::make_unique<Bar>(p);
}

Which seems to emit the same false positive, but not a parsing error.

You are right, your simplified code produces a FP, which I recorded in this ticket: CPP-3545. The root cause is the missing implementation of “Parenthesized initialization of aggregates” in clang 13 that we use as a frontend. Thank you for reporting it!

However, I was not able to reproduce FP on your original code with unique_ptr. There is certainly something in your compiler configuration or surrounding code that I am missing.

To nail down the problem I will need a “reproducer” file that collects all the information necessary for me to reproduce an issue. See these instructions on how to generate it. If you worry about sharing potentially sensitive information from your build environment you can send it to me privately.

As discussed privately, while the reproducer for the original false-positive remains elusive, @torgeir.skogen found another issue, this time a false-negative: CPP-3548.

Thank you again. This thread has been quite productive so far :slight_smile:

Let me know if you manage to create a reproducer for the original issue or have other questions.

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.