Bug 378677 - returning-void-expression false positive
Summary: returning-void-expression false positive
Status: RESOLVED FIXED
Alias: None
Product: clazy
Classification: Developer tools
Component: general (show other bugs)
Version: unspecified
Platform: unspecified All
: NOR normal
Target Milestone: ---
Assignee: Sergio Martins
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-11 23:01 UTC by oktal3700
Modified: 2017-04-13 20:37 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description oktal3700 2017-04-11 23:01:23 UTC
The returning-void-expression warning is spuriously generated by any code that returns a braced-initializer, such as the following testcase:

int test()
{
    return {};
}

In Clang's AST, before semantic analysis, the type of a braced-initializer is void.

The AST visitor should skip the warning if the return value is-a InitListExpr.
Comment 1 Sergio Martins 2017-04-12 14:30:29 UTC
Git commit 9bccb9a02a1ee0211e7e2b4cadd2d785a4ba792b by Sergio Martins.
Committed on 12/04/2017 at 14:29.
Pushed by smartins into branch '1.1'.

Add unit-test for bug 378677

M  +5    -0    tests/returning-void-expression/main.cpp

https://commits.kde.org/clazy/9bccb9a02a1ee0211e7e2b4cadd2d785a4ba792b
Comment 2 Sergio Martins 2017-04-12 14:33:22 UTC
Which clang version are you on ? I can't repro with 3.9.

`-FunctionDecl 0x1f64cf0 <v.cpp:1:1, line:4:1> line:1:5 test 'int (void)'
  `-CompoundStmt 0x1f64e70 <line:2:1, line:4:1>
    `-ReturnStmt 0x1f64e58 <line:3:5, col:13>
      `-InitListExpr 0x1f64e18 <col:12, col:13> 'int'
Comment 3 oktal3700 2017-04-12 21:02:23 UTC
I'm sorry, I jumped to a conclusion and gave you faulty information without first checking it thoroughly for myself. The false positive occurs when the function is constexpr and the return type is dependent on a template parameter.

template <typename T>
constexpr T test3()
{
    return {}; // OK (bug #378677)
}

I have confirmed that _this_ testcase does exhibit the bug on my system, which is the pre-built Windows binary.
Comment 4 Sergio Martins 2017-04-13 16:35:25 UTC
Git commit de9f826e305db1baee7774db44b318e33d5145a4 by Sergio Martins.
Committed on 13/04/2017 at 16:33.
Pushed by smartins into branch '1.1'.

returning-void-expression: Don't warn for templates returning T

M  +13   -0    src/ContextUtils.h
M  +10   -0    src/checks/level2/returning-void-expression.cpp
M  +12   -0    tests/returning-void-expression/main.cpp
M  +1    -0    tests/returning-void-expression/main.cpp.expected

https://commits.kde.org/clazy/de9f826e305db1baee7774db44b318e33d5145a4
Comment 5 oktal3700 2017-04-13 17:45:35 UTC
The unittest in the commit doesn't reproduce the bug on my system, as the function templates are not instantiated. Only after adding the following does it really test the bug:

void test6()
{
    test4<int>();
    test5<int>();
}

The other way I found, without adding test6, was to make test4 and test5 constexpr.
Comment 6 Sergio Martins 2017-04-13 18:18:45 UTC
Git commit dcc9f740b83c1838f22981990016880295b62203 by Sergio Martins.
Committed on 13/04/2017 at 18:18.
Pushed by smartins into branch '1.1'.

returning-void-expression: Add more unit-test

These also pass now.

M  +9    -1    tests/returning-void-expression/main.cpp
M  +1    -1    tests/returning-void-expression/main.cpp.expected

https://commits.kde.org/clazy/dcc9f740b83c1838f22981990016880295b62203
Comment 7 Sergio Martins 2017-04-13 18:19:58 UTC
Strange, since I reproduced the bug on my system without needing to instantiate.
Anyway, the test not covers all cases :)
Comment 8 oktal3700 2017-04-13 20:37:37 UTC
Of course it can not cover all cases, but it should at least cover one case. ;)

Looks good now, thank you.