Whole Tomato Software Forums
Whole Tomato Software Forums
Main Site | Profile | Register | Active Topics | Members | Search | FAQ
User name:
Password:
Save Password
Forgot your password?

 All Forums
 Visual Assist
 Technical Support
 Code Inspection
 New Topic  Reply to Topic
 Printer Friendly
Author Previous Topic Topic Next Topic  

jakeslade
New Member

United Kingdom
2 Posts

Posted - Oct 01 2019 :  07:54:53 AM  Show Profile  Reply with Quote
Hello,

I've been considering a purhase of VAX license. I read about Clang integration and this made me excited to try out Code Inspection features.

The result is disappointing. It doesn't pick up anything other than the most basic of Clang's suggestions.

For example, it doesn't even suggest range based for loop improvements in the most basic of circumstances.

The following example would be picked up by Clang, but it isn't picked up by VAX's Code Inspection:

https://pastebin.com/txbfNSGM

Is there something I need to configure for it to work properly? If not, is this feature likely to be improved?

Cheers

Edited by - jakeslade on Oct 01 2019 07:58:01 AM

feline
Whole Tomato Software

United Kingdom
16105 Posts

Posted - Oct 01 2019 :  10:35:17 AM  Show Profile  Reply with Quote
Which IDE and version of VA are you using?

Can you please try adding the following code to one of your cpp files, and see if Code Inspection detects this loop, and suggests updating it? It does so for me, using VS2017 and VA 2341.2:

static int forLoopAcrossConst(const std::map<std::string, short> &mapNamesToNumbers)
{
	int nTotal;
	for (auto mapIt = mapNamesToNumbers.begin(); mapIt != mapNamesToNumbers.end(); ++mapIt)
	{
		nTotal += mapIt->second;
	}
}

looking at your loop, you have a comma inside the for statement, along with 2 semi-colons. Is this deliberate?

From the names used I can see what the idea is here, but I don't recognise this syntax - *edit* you are declaring two variables here, of the same type, separated by a comma. I just don't remember seeing this done before in this context, but it makes sense once I recognise it.

zen is the art of being at one with the two'ness

Edited by - feline on Oct 01 2019 10:39:09 AM
Go to Top of Page

jakeslade
New Member

United Kingdom
2 Posts

Posted - Oct 01 2019 :  12:29:36 PM  Show Profile  Reply with Quote
Interesting, it did pick that up and suggest updating.

I changed to auto to explicit iterator type and it no longer picks it up:

static int forLoopAcrossConst(const std::map<std::string, short> &mapNamesToNumbers)
{
	int nTotal;
	for (std::map<std::string, short>::iterator mapIt = mapNamesToNumbers.begin(); mapIt != mapNamesToNumbers.end(); ++mapIt)
	{
		nTotal += mapIt->second;
	}
}


I'm using VS2015 Pro and VA 2341.2

Appreciate the swift reply!

Edited by - jakeslade on Oct 01 2019 12:30:02 PM
Go to Top of Page

feline
Whole Tomato Software

United Kingdom
16105 Posts

Posted - Oct 01 2019 :  1:01:16 PM  Show Profile  Reply with Quote
Does this version of the for loop compile for you? It is failing to compile for me, still in VS2017, even though on the surface it looks quite reasonable... if the code is invalid, its not a surprise that Code Inspection is not suggesting anything.

zen is the art of being at one with the two'ness
Go to Top of Page

sean
Whole Tomato Software

USA
2711 Posts

Posted - Oct 01 2019 :  1:34:56 PM  Show Profile  Reply with Quote
Odd -- the inspection doesn't light up until the body of the for loop uses the start variable:
int x = *start;

Edited by - sean on Oct 01 2019 1:37:25 PM
Go to Top of Page

feline
Whole Tomato Software

United Kingdom
16105 Posts

Posted - Oct 04 2019 :  6:05:59 PM  Show Profile  Reply with Quote
I am now organised to test Clang directly, to compare what VA is doing to what Clang does. Your original code is:

#include <vector>
 
int main()
{
    std::vector<int> v = {1, 2, 3};
 
    for(std::vector<int>::const_iterator start = v.begin(), end = v.end(); start != end; ++start)
    {
    }
 
    int *pInt = 0;
}

I have tested this code, using the command:

clang-tidy code_test.cpp -checks=-*,modernize-* --

on both Ubuntu Linux with clang-tidy version 6.0.0 and on Windows 10 with clang-tidy version 9.0.0, and neither test reports that the for loop can be changed. Have you actually tested this code with clang-tidy, and if so, which version, and what command line switches did you use?

zen is the art of being at one with the two'ness
Go to Top of Page

feline
Whole Tomato Software

United Kingdom
16105 Posts

Posted - Oct 05 2019 :  09:22:24 AM  Show Profile  Reply with Quote
For the other loop, VA is not picking up that the for loop can be converted with the code:

static int forLoopAcrossConst(const std::map<std::string, short> &mapNamesToNumbers)
{
	int nTotal;
	for (std::map<std::string, short>::iterator mapIt = mapNamesToNumbers.begin(); mapIt != mapNamesToNumbers.end(); ++mapIt)
	{
		nTotal += mapIt->second;
	}
}

but this contains an error, it won't compile. You need to change the "iterator" into a "const_iterator", obvious when you spot it, since the map parameter is a const parameter. This gives you the loop:

static int forLoopAcrossConst(const std::map<std::string, short> &mapNamesToNumbers)
{
	int nTotal;
	for (std::map<std::string, short>::const_iterator mapIt = mapNamesToNumbers.begin(); mapIt != mapNamesToNumbers.end(); ++mapIt)
	{
		nTotal += mapIt->second;
	}
}

and VA Code Inspection is picking up that this loop can be converted quite happily, and the fix works correctly when applied.

zen is the art of being at one with the two'ness
Go to Top of Page
  Previous Topic Topic Next Topic  
 New Topic  Reply to Topic
 Printer Friendly
Jump To:
© 2019 Whole Tomato Software, LLC Go To Top Of Page
Snitz Forums 2000