Page MenuHomePhabricator

[clang-tidy] Add a check for constructing non-trivial types without assigning to a variable
Needs RevisionPublic

Authored by pfultz2 on Mar 7 2018, 6:01 PM.

Details

Reviewers
aaron.ballman
hokein
alexfh
Group Reviewers
Restricted Project
Summary

Diagnoses when a non-trivial type is not assigned to a variable. This is useful to check for problems like unnamed lock guards.

For example, if you had code written like this:

int g_i = 0;
std::mutex g_i_mutex;  // protects g_i
 
void safe_increment()
{
  std::lock_guard<std::mutex>{g_i_mutex};
  // The lock is locked and then unlocked before reaching here
  ++g_i;
}

This will warn that a variable should be created instead:

int g_i = 0;
std::mutex g_i_mutex;  // protects g_i
 
void safe_increment()
{
  std::lock_guard<std::mutex> lock{g_i_mutex};
  // The lock is locked and then will be unlocked when exiting scope
  ++g_i;
}

Diff Detail

Event Timeline

pfultz2 created this revision.Mar 7 2018, 6:01 PM

I'd like to see some more tests

void valid() {
  g(NonTrivial{}); // Should not warn
}

Have you already tried running it on some C++ codebase (llvm, qt, boost, ...) ?
What are the results?

JonasToth retitled this revision from Add a check for constructing non-trivial types without assigning to a variable to [clang-tidy] Add a check for constructing non-trivial types without assigning to a variable.Mar 8 2018, 3:11 AM
JonasToth added reviewers: hokein, alexfh.
alexfh added a comment.Mar 8 2018, 4:30 AM

How does this differ from misc-unused-raii?

alexfh requested changes to this revision.Mar 8 2018, 4:30 AM
This revision now requires changes to proceed.Mar 8 2018, 4:30 AM

How does this differ from misc-unused-raii?

Actually, I was unaware of this check. This seems like a better place to start. Whats missing from the misc-unused-raii is that it doesn't catch it when constructing with curly braces(ie NonTrivial{}). Let me work on updating that check instead.