Whole Tomato Software Forums
Whole Tomato Software Forums
Main Site | Profile | Register | Active Topics | Members | Search | FAQ
 All Forums
 Visual Assist
 Technical Support
 Code Inspection

You must be registered to post a reply.
Click here to register.

Screensize:
UserName:
Password:
Format: BoldItalicizeUnderlineStrikethrough Align leftCenterAlign right Insert horizontal ruleUpload and insert imageInsert hyperlinkInsert email addressInsert codeInsert quoted textInsert listInsert Emoji
   
Message:

Forum code is on.
Html is off.

 
Check to subscribe to this topic.
   

T O P I C    R E V I E W
jakeslade Posted - Oct 01 2019 : 07:54:53 AM
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
6   L A T E S T    R E P L I E S    (Newest First)
feline Posted - Oct 05 2019 : 09:22:24 AM
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.
feline Posted - Oct 04 2019 : 6:05:59 PM
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?
sean Posted - Oct 01 2019 : 1:34:56 PM
Odd -- the inspection doesn't light up until the body of the for loop uses the start variable:
int x = *start;
feline Posted - Oct 01 2019 : 1:01:16 PM
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.
jakeslade Posted - Oct 01 2019 : 12:29:36 PM
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!
feline Posted - Oct 01 2019 : 10:35:17 AM
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.

© 2023 Whole Tomato Software, LLC Go To Top Of Page
Snitz Forums 2000