| T O P I C R E V I E W |
| azur |
Posted - Sep 27 2022 : 04:01:27 AM
The code issue & checker Find manually-defined constructor definitions that do not initialize all fields could be more helpful if the fields leading to this match are displayed.
If the word "field" were then pronounced as a link in the tooltip, the fields could then be displayed in a dialog.
Maybe I just don't know a hotkey to display this information right now.
|
| 16 L A T E S T R E P L I E S (Newest First) |
| feline |
Posted - Sep 22 2023 : 06:01:35 AM
There is quite a bit of documentation to read, to be fair 
I actually don't know why the examples are pictures, that does seem unhelpful.
I do have the free Microsoft Powertoys installed:
https://github.com/microsoft/PowerToys
which has a whole pile of utilities in it, one of which is optical character recognition, very helpful when people post screen shots of code to support. It's not perfect, but it saves a whole load of re-typing. Just something to be aware of if you run into needing to extract text from pictures often.
At least here the skip commands are short and simple. They only get longer if you want to exclude specific checks by name, but thankfully the names are listed here:
https://docs.wholetomato.com/default.asp?W761
in the second column, LLVM/Clang Equivalent and Documentation, so you can copy and paste them, if you want to make your skip comments very precise. I would suggest this personally, simply because other things may be caught going forward, that you want to be warned about, and if code is skipped it won't be checked. |
| azur |
Posted - Sep 21 2023 : 1:14:03 PM
Thanks. That's very helpful. (I should have read the documentation to the end ;-)
But why are there example pictures? So you have to copy them from the screen by hand in order to have them in a text file. |
| feline |
Posted - Sep 21 2023 : 12:41:52 PM
SvenR, would it be possible to get a copy of the header file ONLY that is showing this problem? This is the problem we have seen, and clearly need to fix, but so far we only have one example, that was happening in a MFC project. So it would be good to see another example, especially if this is not happening in a MFC project.
If this is possible, and I understand it often isn't, please send me the files via email:
[email protected]
including this thread ID or URL in the description, so we can match it up.
azur, yes, you can skip specific checks on specific pieces of code. This is explained in detail in the section "Disabling Code Inspections for Specific Code Sections" on this page:
https://support.wholetomato.com/default.asp?W760
here is an example of it in action. I have added the skip comment above my constructor, using the clang-tidy name of the code inspection, so only this specific check is being skipped, and the check is being skipped for 2 lines of code, which is enough in this case.
// vaCI:skip+2:cppcoreguidelines-pro-type-member-init
TestingSimpleStringInit()
{
rawCharArray[0] = 0;
} |
| azur |
Posted - Sep 21 2023 : 12:14:21 PM
quote:
Originally posted by feline
WCHAR m_szText[20];
to:
WCHAR m_szText[20]{};
Is there a way to tell the examiner that this is ok? The hint about a possible problem is very good. But why should I initialize a large CHAR array if I'm just using it as an intermediate buffer? The background is simply an attempt to minimize reallocations.
A kind of �compiler hint� would be helpful here. |
| SvenR |
Posted - Sep 21 2023 : 12:00:02 PM
quote:
Originally posted by feline
Do you see the same results on your system? If the {} are being inserted into the correct place then that's one thing. But if they are being placed in the wrong place then we definitely have a problem.
Yes with the sample. But with my original code, the position could be improved a little:
|
| feline |
Posted - Sep 21 2023 : 10:58:19 AM
OK, I can reproduce the code inspection warning with a simple char array, no need to use WCHAR. It turns out the warning is "by design", from the page:
https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/pro-type-member-init.html
clang-tidy, which is what we are using as the basis for code inspections, reports this as a problem since not all members of the array are being set. If the array was being used as anything other than a NULL terminated string then this is actually a valid report.
So it depends on how the static code analysis is treating these arrays. Given the variable name it is "obvious" to me that this is going to be a NULL terminated string, but I can certainly see someone using a char array for something other than a NULL terminated string. In fact I am sure I have done that myself at some point over the years, since if you need an array that is guaranteed to only store small numbers (under 100), it makes a sort of sense.
Accepting the code fix, for me, concerts the array initialization from:
WCHAR m_szText[20];
to:
WCHAR m_szText[20]{};
which still compiles quite happily for me. Now the constructor wasn't updated, but in this case I don't think that matters.
Do you see the same results on your system? If the {} are being inserted into the correct place then that's one thing. But if they are being placed in the wrong place then we definitely have a problem. |
| SvenR |
Posted - Sep 21 2023 : 10:00:51 AM
I have some classes where the hint 'Manually defined constructor does not initialize all fields' is displayed. However, all variables are initialized and the static code analysis does not show any errors.
The cause seems to be the initialization of strings:
class CTest {
WCHAR m_szText[20];
public:
CTest()
{
m_szText[0] = 0;
}
};
|
| feline |
Posted - Sep 21 2023 : 07:58:25 AM
Would I be able to get a copy of a header file that shows this problem? I appreciate this is often not possible, but it's worth asking on the off chance it is possible.
If this is possible, please send me the files via email:
[email protected]
including this thread ID or URL in the description, so we can match it up.
I have tested this here with multiple classes in one file myself, and that isn't, on its own, enough to trigger the problem for me. So I would be very interested in having a look at a test case that shows the problem happening just from this, since that sounds like a rather common, and easy to encounter, trigger. |
| azur |
Posted - Sep 21 2023 : 06:33:52 AM
This shouldn't have anything to do with the MFC. I think the problem is that there are multiple classes in one file. |
| feline |
Posted - Sep 21 2023 : 06:11:00 AM
Apologies for this problem. Are you working with MFC, or something else? We have only seen a couple of examples of this problem so far, and both of those are when working in MFC, so if this is happening somewhere else as well then that is a concern.
I have increased the priority on this bug report, but unfortunately I don't currently have an estimate for when it will be fixed. |
| ian.gralinski |
Posted - Sep 20 2023 : 8:57:29 PM
I am also seeing this issue occur whereby the braces are being inserted into seemingly random locations in the code.
Is there a tentative date when a fix for this issue will be released? |
| feline |
Posted - Sep 29 2022 : 12:51:06 PM
The trigger seems to be something in the project structure, not the code in the Dlg files. I have simplified the Dlg files down to basically nothing and still get the problem. Very strange, but at least it is a clue. Thank you again for reporting this nasty bug.
As for knowing what is going to happen, we are considering showing a preview of the code change that code inspection will create:
case=104512
which would help here. When this code inspection is working correctly the code edit will always be where the class members are declared, so all of the edits will occur together, only inside the class declaration, leaving all of the constructors untouched. |
| azur |
Posted - Sep 28 2022 : 10:06:28 AM
Thanks for working on this issue.
Perhaps a query with the affected field(s) can be placed before the fix?
If I already have a constructor with a lot of assignments, I might want to add the missing field(s) myself. Otherwise, the VA fix might give me a mixture of initializations in the declaration (H) and implementation (CPP).
|
| feline |
Posted - Sep 28 2022 : 09:47:29 AM
Ouch, thank you for the clear description, I am seeing the same problem here. The exact placement of the brackets seems to vary a bit, but the general problem is very clear:
case=148676
Now to see if I can make a really small test case that shows the problem... |
| azur |
Posted - Sep 27 2022 : 08:34:34 AM
The inspection fix not worked as espected. Maybe there is a problem with multiple classes in one file.
Try this:
create new MFC-Dialog-Project
in *Dlg.h add a Field like this
HICON m_hIcon;
UINT m_nCount;
switch to *Dlg.cpp
The constructor *Dlg::*Dlg got the yellow inspection mark
try to fix
In my case it's insert curly braces somewhere in the source code:
void CAboutDlg::DoDataExchange(CDataExchange*{} pDX)
{
CDialogEx::DoDataExchange(pDX);
}
|
| feline |
Posted - Sep 27 2022 : 07:33:50 AM
The code inspection is designed to do the initialization for you, automatically. Consider the following example:
class TestCodeInspectConstructorInitSimpleFields
{
char m_cCharData;
short m_nShortData;
int m_nIntData;
long m_nLongData;
float m_fFloatData;
double m_fDoubleData;
// underlined, initializes all members above #CodeInspectConstructorInitAll
TestCodeInspectConstructorInitSimpleFields() { }
// underlined, initializes three members above #CodeInspectConstructorInitAll
TestCodeInspectConstructorInitSimpleFields(bool bInitSome) : m_cCharData('0'), m_nShortData(0), m_fFloatData(0.1f) {}
// not underlined here, since all fields are initalized
TestCodeInspectConstructorInitSimpleFields(std::string strInitAll)
{
m_cCharData = '0';
m_nShortData = 0;
m_nIntData = 0;
m_nLongData = 0;
m_fFloatData = 0.1f;
m_fDoubleData = 0.2f;
}
};
trigger the code inspection fix on either of the first two constructors, and the members that this constructor does not initialize are initialized at declaration with default values, leaving the constructor unchanged.
Are you looking for a list of fields to manually initialize for a reason? |
