<div dir="ltr">Submitted as r207715</div><div class="gmail_extra"><br clear="all"><div><span style="color:rgb(153,153,153)">-Alex</span><br></div>
<br><br><div class="gmail_quote">On Wed, Apr 30, 2014 at 3:31 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<p dir="ltr">LGTM</p><div class="HOEnZb"><div class="h5">
<div class="gmail_quote">On 30 Apr 2014 15:24, "Alex McCarthy" <<a href="mailto:alexmc@google.com" target="_blank">alexmc@google.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">Fix pending in <a href="http://reviews.llvm.org/D3578" target="_blank">http://reviews.llvm.org/D3578</a> . Mind taking a look?<br><br><div>Thanks for your keen eyes!</div></div><div class="gmail_extra"><br clear="all">
<div>
<span style="color:rgb(153,153,153)">-Alex</span><br></div>
<br><br><div class="gmail_quote">On Wed, Apr 30, 2014 at 3:08 PM, Alex McCarthy <span dir="ltr"><<a href="mailto:alexmc@google.com" target="_blank">alexmc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">Oops, good catch, I'll get right on that. (Sorry about that: my C++ is rusty, I forgot that members aren't safely default initialized)</div><div class="gmail_extra"><span><font color="#888888"><br clear="all">
<div><span style="color:rgb(153,153,153)">-Alex</span><br>
</div></font></span><div><div>
<br><br><div class="gmail_quote">On Wed, Apr 30, 2014 at 2:51 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div>On Wed, Apr 30, 2014 at 7:09 AM, Alex McCarthy <span dir="ltr"><<a href="mailto:alexmc@google.com" target="_blank">alexmc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: alexmc<br>
Date: Wed Apr 30 09:09:24 2014<br>
New Revision: 207652<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=207652&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=207652&view=rev</a><br>
Log:<br>
Add a clang-tidy flag to support temporary destructor-aware analysis (workaround for bug 15599).<br>
<br>
Reviewers: alexfh<br>
<br>
Subscribers: jordan_rose, klimek, djasper, cfe-commits<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D3556" target="_blank">http://reviews.llvm.org/D3556</a><br>
<br>
Added:<br>
clang-tools-extra/trunk/test/clang-tidy/temporaries.cpp<br>
Modified:<br>
clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp<br>
clang-tools-extra/trunk/clang-tidy/ClangTidy.h<br>
clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.h<br>
clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=207652&r1=207651&r2=207652&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=207652&r1=207651&r2=207652&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Wed Apr 30 09:09:24 2014<br>
@@ -173,8 +173,9 @@ private:<br>
} // namespace<br>
<br>
ClangTidyASTConsumerFactory::ClangTidyASTConsumerFactory(<br>
- ClangTidyContext &Context)<br>
- : Context(Context), CheckFactories(new ClangTidyCheckFactories) {<br>
+ ClangTidyContext &Context, const ClangTidyOptions &Options)<br>
+ : Context(Context), CheckFactories(new ClangTidyCheckFactories),<br>
+ Options(Options) {<br>
for (ClangTidyModuleRegistry::iterator I = ClangTidyModuleRegistry::begin(),<br>
E = ClangTidyModuleRegistry::end();<br>
I != E; ++I) {<br>
@@ -207,16 +208,21 @@ clang::ASTConsumer *ClangTidyASTConsumer<br>
if (!CheckFactories->empty())<br>
Consumers.push_back(Finder.newASTConsumer());<br>
<br>
- AnalyzerOptionsRef Options = Compiler.getAnalyzerOpts();<br>
- Options->CheckersControlList = getCheckersControlList();<br>
- if (!Options->CheckersControlList.empty()) {<br>
- Options->AnalysisStoreOpt = RegionStoreModel;<br>
- Options->AnalysisDiagOpt = PD_NONE;<br>
- Options->AnalyzeNestedBlocks = true;<br>
- Options->eagerlyAssumeBinOpBifurcation = true;<br>
+ AnalyzerOptionsRef AnalyzerOptions = Compiler.getAnalyzerOpts();<br>
+ // FIXME: Remove this option once clang's cfg-temporary-dtors option defaults<br>
+ // to true.<br>
+ AnalyzerOptions->Config["cfg-temporary-dtors"] =<br>
+ Options.AnalyzeTemporaryDtors ? "true" : "false";<br>
+<br>
+ AnalyzerOptions->CheckersControlList = getCheckersControlList();<br>
+ if (!AnalyzerOptions->CheckersControlList.empty()) {<br>
+ AnalyzerOptions->AnalysisStoreOpt = RegionStoreModel;<br>
+ AnalyzerOptions->AnalysisDiagOpt = PD_NONE;<br>
+ AnalyzerOptions->AnalyzeNestedBlocks = true;<br>
+ AnalyzerOptions->eagerlyAssumeBinOpBifurcation = true;<br>
ento::AnalysisASTConsumer *AnalysisConsumer = ento::CreateAnalysisConsumer(<br>
Compiler.getPreprocessor(), Compiler.getFrontendOpts().OutputFile,<br>
- Options, Compiler.getFrontendOpts().Plugins);<br>
+ AnalyzerOptions, Compiler.getFrontendOpts().Plugins);<br>
AnalysisConsumer->AddDiagnosticConsumer(<br>
new AnalyzerDiagnosticConsumer(Context));<br>
Consumers.push_back(AnalysisConsumer);<br>
@@ -288,7 +294,7 @@ void ClangTidyCheck::setName(StringRef N<br>
std::vector<std::string> getCheckNames(const ClangTidyOptions &Options) {<br>
SmallVector<ClangTidyError, 8> Errors;<br>
clang::tidy::ClangTidyContext Context(&Errors, Options);<br>
- ClangTidyASTConsumerFactory Factory(Context);<br>
+ ClangTidyASTConsumerFactory Factory(Context, Options);<br>
return Factory.getCheckNames();<br>
}<br>
<br>
@@ -326,7 +332,7 @@ void runClangTidy(const ClangTidyOptions<br>
ClangTidyASTConsumerFactory *ConsumerFactory;<br>
};<br>
<br>
- Tool.run(new ActionFactory(new ClangTidyASTConsumerFactory(Context)));<br>
+ Tool.run(new ActionFactory(new ClangTidyASTConsumerFactory(Context, Options)));<br>
}<br>
<br>
void handleErrors(SmallVectorImpl<ClangTidyError> &Errors, bool Fix) {<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.h?rev=207652&r1=207651&r2=207652&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.h?rev=207652&r1=207651&r2=207652&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.h (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.h Wed Apr 30 09:09:24 2014<br>
@@ -94,7 +94,8 @@ class ClangTidyCheckFactories;<br>
<br>
class ClangTidyASTConsumerFactory {<br>
public:<br>
- ClangTidyASTConsumerFactory(ClangTidyContext &Context);<br>
+ ClangTidyASTConsumerFactory(ClangTidyContext &Context,<br>
+ const ClangTidyOptions &Options);<br>
~ClangTidyASTConsumerFactory();<br>
<br>
/// \brief Returns an ASTConsumer that runs the specified clang-tidy checks.<br>
@@ -112,6 +113,7 @@ private:<br>
ClangTidyContext &Context;<br>
ast_matchers::MatchFinder Finder;<br>
std::unique_ptr<ClangTidyCheckFactories> CheckFactories;<br>
+ ClangTidyOptions Options;<br>
};<br>
<br>
/// \brief Fills the list of check names that are enabled when the provided<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.h?rev=207652&r1=207651&r2=207652&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.h?rev=207652&r1=207651&r2=207652&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.h (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.h Wed Apr 30 09:09:24 2014<br>
@@ -18,6 +18,7 @@ struct ClangTidyOptions {<br>
ClangTidyOptions() : EnableChecksRegex(".*") {}<br>
std::string EnableChecksRegex;<br>
std::string DisableChecksRegex;<br>
+ bool AnalyzeTemporaryDtors;<br></blockquote><div><br></div></div></div><div>It looks like default-constructed state of ClangTidyOptions is supposed to be a valid state. Please update the default constructor to initialize this member!<br>
</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
};<br>
<br>
} // end namespace tidy<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp?rev=207652&r1=207651&r2=207652&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp?rev=207652&r1=207651&r2=207652&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp Wed Apr 30 09:09:24 2014<br>
@@ -46,12 +46,20 @@ static cl::opt<bool> ListChecks("list-ch<br>
cl::desc("List all enabled checks and exit."),<br>
cl::init(false), cl::cat(ClangTidyCategory));<br>
<br>
+static cl::opt<bool> AnalyzeTemporaryDtors(<br>
+ "analyze-temporary-dtors",<br>
+ cl::desc("Enable temporary destructor-aware analysis in clang-analyzer- "<br>
+ "checks."),<br>
+ cl::init(false),<br>
+ cl::cat(ClangTidyCategory));<br>
+<br>
int main(int argc, const char **argv) {<br>
CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory);<br>
<br>
clang::tidy::ClangTidyOptions Options;<br>
Options.EnableChecksRegex = Checks;<br>
Options.DisableChecksRegex = DisableChecks;<br>
+ Options.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;<br>
<br>
// FIXME: Allow using --list-checks without positional arguments.<br>
if (ListChecks) {<br>
<br>
Added: clang-tools-extra/trunk/test/clang-tidy/temporaries.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/temporaries.cpp?rev=207652&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/temporaries.cpp?rev=207652&view=auto</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/test/clang-tidy/temporaries.cpp (added)<br>
+++ clang-tools-extra/trunk/test/clang-tidy/temporaries.cpp Wed Apr 30 09:09:24 2014<br>
@@ -0,0 +1,26 @@<br>
+// RUN: clang-tidy -checks=clang-analyzer-core.NullDereference -disable-checks='' -analyze-temporary-dtors %s -- > %t.log<br>
+// FileCheck complains if the input file is empty, so add a dummy line.<br>
+// RUN: echo foo >> %t.log<br>
+// RUN: FileCheck %s < %t.log<br>
+<br>
+struct NoReturnDtor {<br>
+ ~NoReturnDtor() __attribute__((noreturn));<br>
+};<br>
+<br>
+extern bool check(const NoReturnDtor &);<br>
+<br>
+// CHECK-NOT: warning<br>
+void testNullPointerDereferencePositive() {<br>
+ int *value = 0;<br>
+ // CHECK: [[@LINE+1]]:10: warning: Dereference of null pointer (loaded from variable 'value') [clang-analyzer-core.NullDereference]<br>
+ *value = 1;<br>
+}<br>
+<br>
+// CHECK-NOT: warning<br>
+void testNullPointerDereference() {<br>
+ int *value = 0;<br>
+ if (check(NoReturnDtor())) {<br>
+ // This unreachable code causes a warning if we don't run with -analyze-temporary-dtors<br>
+ *value = 1;<br>
+ }<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div></div>
</blockquote></div><br></div>
</blockquote></div>
</div></div></blockquote></div><br></div>