CPP: Exclude template code from ExprHasNoEffect.ql

[geoffw0]

Exclude expressions inside template instantiations from ExprHasNoEffect.ql. These are usually false positives due to the expression in question having an effect in other instantiations (real or hypothetical).

[jbj]

Why remove the macro check? Is it redundant because parent.isInMacroExpansion() is checked below?

[geoffw0]

Hmm, I didn't really intend to remove it. I was curious whether that line affected any of the tests, and it didn't, but that doesn't necessarily mean it's truly doing nothing. I'll add some test cases for the relationship between this and parent.isInMacroExpansion() and probably reinstate it.

[geoffw0]

I've added a test case for macros and reinstated the macro check. I now think it's actually the other macro clause that might be redundant - but with no explanatory comments or test cases, and few real world results that care, I'm not confident.

[jbj]

The test failed. I wonder why the Azure tests didn't run on this PR.

11:32:40 Diff:
11:32:40 --- expected
11:32:40 +++ actual
11:32:40 @@ -1,6 +1,5 @@
11:32:40  | calls.cpp:8:5:8:5 | 1 | This expression has no effect. | calls.cpp:8:5:8:5 | 1 |  |
11:32:40  | calls.cpp:12:5:12:16 | call to thingy | This expression has no effect (because $@ has no external side effects). | calls.cpp:7:15:7:20 | thingy | thingy |
11:32:40 -| templatey.cpp:4:3:4:8 | ... << ... | This expression has no effect. | templatey.cpp:4:3:4:8 | ... << ... |  |
11:32:40  | templatey.cpp:39:3:39:23 | call to pointless_add_numbers | This expression has no effect (because $@ has no external side effects). | templatey.cpp:29:5:29:25 | pointless_add_numbers | pointless_add_numbers |
11:32:40  | volatile.c:9:5:9:5 | c | This expression has no effect. | volatile.c:9:5:9:5 | c |  |
11:32:40  | volatile.c:12:5:12:9 | access to array | This expression has no effect. | volatile.c:12:5:12:9 | access to array |  |

[dave-bartolomeo]

The Azure tests did run (and caught the failure): https://semmle.visualstudio.com/QL/_build/results?buildId=680&view=logs
What I don't know is why they didn't show up in the checks list at all.

[geoffw0]

It appears we have some additional query tests hiding in ql\cpp\ql\test\library-tests\queries. For this PR I will just update the .expected file in question (the change is as we'd expect). Next week I'll open a PR to move / merge these tests into ql\cpp\ql\test\query-tests where they should be.