Page MenuHomePhabricator

[Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash
ClosedPublic

Authored by george.karpenkov on Oct 17 2017, 11:44 AM.

Details

Summary

Remove an option to use a reference type (on by default!) since a non-reference type is always needed for creating expressions, functions with multiple boolean parameters are very hard to use, and in general it was just a booby trap for further crashes.
Furthermore, generalize call_once test case to fix some of the crashes mentioned https://bugs.llvm.org/show_bug.cgi?id=34869

Diff Detail

Repository
rL LLVM

Event Timeline

dcoughlin accepted this revision.Oct 17 2017, 3:26 PM

Looks good to me.

This comment is unrelated to this particular patch, but since I know you're doing other work in the area I think it would also be good to have test cases for the two other forms of call_once defined in <mutex>. It looks to me like those will be used for projects that that use pre-C++11 -- so we should have test coverage for them.

This revision is now accepted and ready to land.Oct 17 2017, 3:26 PM
This revision was automatically updated to reflect the committed changes.
danielmarjamaki added inline comments.
cfe/trunk/lib/Analysis/BodyFarm.cpp
139

this looks strange, did clang-format do this?

alexfh added a subscriber: alexfh.Oct 24 2017, 3:54 PM
alexfh added inline comments.
cfe/trunk/lib/Analysis/BodyFarm.cpp
366

Remove the spaces inside the comment. /*ParamName=*/value is the format that is understood by clang-format and the https://clang.llvm.org/extra/clang-tidy/checks/misc-argument-comment.html clang-tidy check. Same elsewhere in this patch.

cfe/trunk/lib/Analysis/BodyFarm.cpp
139

yes i believe so.

366

yeah, i can do that.
can we also consider changing clang-tidy to ignore those spaces?

alexfh added inline comments.Oct 25 2017, 7:19 AM
cfe/trunk/lib/Analysis/BodyFarm.cpp
366

I've just looked at the source code, and, apparently, clang-tidy already understands spaces in argument comments already. Sorry for misinforming you. However, clang-format is more strict here and there's also the consistency argument (see the comment on r316539), which I find to be a good motivation to change this (or at least to not introduce more inconsistency).