Uploaded image for project: 'SonarCFamily'
  1. SonarCFamily
  2. CPP-3011

Rule S6231: "std::string_view" and "std::span" parameters should be directly constructed from sequences

    XMLWordPrintable

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 6.18
    • Fix Version/s: 6.26
    • Component/s: C++, Rules
    • Labels:

      Description

      After applying the advice of RSPEC-6188 about changing the parameter type from a std::vector<T const*> to std::span<T const*> the developer might forget to remove a temporary used in the call site to satisfy the original interface (i.e., a copy of an std::vector<T*> with the proper type). It will work just fine but have an unnecessary copy.

      A straightforward way to suggest a fix at the call site as a secondary location when reporting the main issue (with primary location on the function signature) in RSPEC-6188 is not feasible.
      It would require either a project-level rule to enumerate all call sites in one report, or reporting just one call site, leaving other call sites behind, and creating instability in the SQ report because it would depend on the analysis order of translation units.

      Additionally, new call sites might be imported into the project after the fix but written to the original interface. They will be missed in case we only suggest call-site optimization together with changing the function signature. This may also happen when the called function is the part of the third-party library that got interface modernized with a newer version.

      This should also cover the situation when temporary std::string is created for the function that accepts std::string_view, for example, when substr is called on the std::string, before calling the function.

      Decide whether we need to create a new rule for this situation or piggy-back it on RSPEC-6188.

      Noncompliant code example

      bool oddAre0(const std::span<int const*>& nums);
      std::vector<int*> getNums();
      void caller() {
        std::vector<int*> nums = getNums();
        if (oddAre0(std::vector<int const*>{nums.begin(), nums.end()})) { // Noncompliant: This copy is verbose and slow
          // ...
        }
      }
      
      bool part(std::string_view sv);
      
      void caller2(std::string const& s)
      {
         part(s.substr(10));
      }
      
      

      Compliant solution

      bool oddAre0(const std::span<int const*>& nums);
      std::vector<int*> getNums();
      void caller() {
        std::vector<int*> nums = getNums();
        if (oddAre0(nums)) { // Compliant
          // ...
        }
      }
      
      
      bool part(std::string_view sv);
      
      void caller2(std::string const& s)
      {
         part(std::string_view(s).substr(10));
      }
      

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              tomasz.kaminski Tomasz KamiƄski
              Reporter:
              arseniy.zaostrovnykh Arseniy Zaostrovnykh
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: