Ka-Ka created this revision.
Herald added a subscriber: cfe-commits.
Changed the assert and corresponding warning message for conflicting fixes
to only trigger if clang-tidy is run with -fix or -fix-errors.
The Fix and FixErrors variables are now passed as parameters when creating
respective objects.
This is not a finished patch. We need input if there are a more appropriate fix
of this problem.
Patch by: Henric Karlsson and Björn Pettersson
Repository:
rC Clang
https://reviews.llvm.org/D55438
Files:
lib/StaticAnalyzer/Core/ExprEngineC.cpp
tools/extra/clang-tidy/ClangTidy.cpp
tools/extra/clang-tidy/ClangTidy.h
tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.h
tools/extra/clang-tidy/tool/ClangTidyMain.cpp
tools/extra/test/clang-tidy/clang-tidy-logicalexpr-crash.c
Index: tools/extra/test/clang-tidy/clang-tidy-logicalexpr-crash.c
===================================================================
--- /dev/null
+++ tools/extra/test/clang-tidy/clang-tidy-logicalexpr-crash.c
@@ -0,0 +1,7 @@
+// RUN: clang-tidy -checks='-*,clang-analyzer-*' %s
+
+a;
+fn1() {
+ int b = -a ?: a;
+ (b || 0) && 9;
+}
Index: tools/extra/clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- tools/extra/clang-tidy/tool/ClangTidyMain.cpp
+++ tools/extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -426,7 +426,7 @@
AllowEnablingAnalyzerAlphaCheckers);
std::vector<ClangTidyError> Errors =
runClangTidy(Context, OptionsParser.getCompilations(), PathList, BaseFS,
- EnableCheckProfile, ProfilePrefix);
+ (Fix || FixErrors), EnableCheckProfile, ProfilePrefix);
bool FoundErrors = llvm::find_if(Errors, [](const ClangTidyError &E) {
return E.DiagLevel == ClangTidyError::Error;
}) != Errors.end();
Index: tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -226,7 +226,8 @@
class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
public:
ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx,
- bool RemoveIncompatibleErrors = true);
+ bool RemoveIncompatibleErrors = true,
+ bool FixResolutions = false);
// FIXME: The concept of converting between FixItHints and Replacements is
// more generic and should be pulled out into a more useful Diagnostics
@@ -252,6 +253,7 @@
ClangTidyContext &Context;
bool RemoveIncompatibleErrors;
+ bool FixResolutions;
std::vector<ClangTidyError> Errors;
std::unique_ptr<llvm::Regex> HeaderFilter;
bool LastErrorRelatesToUserCode;
Index: tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -33,8 +33,10 @@
public:
ClangTidyDiagnosticRenderer(const LangOptions &LangOpts,
DiagnosticOptions *DiagOpts,
- ClangTidyError &Error)
- : DiagnosticRenderer(LangOpts, DiagOpts), Error(Error) {}
+ ClangTidyError &Error,
+ bool FixResolutions)
+ : DiagnosticRenderer(LangOpts, DiagOpts), Error(Error),
+ FixResolutions(FixResolutions) {}
protected:
void emitDiagnosticMessage(FullSourceLoc Loc, PresumedLoc PLoc,
@@ -81,10 +83,13 @@
llvm::Error Err = Error.Fix[Replacement.getFilePath()].add(Replacement);
// FIXME: better error handling (at least, don't let other replacements be
// applied).
- if (Err) {
+ if (Err && FixResolutions) {
llvm::errs() << "Fix conflicts with existing fix! "
<< llvm::toString(std::move(Err)) << "\n";
assert(false && "Fix conflicts with existing fix!");
+ } else if (Err) {
+ // Replacements wont be used for fixing anything.
+ consumeError(std::move(Err));
}
}
}
@@ -104,6 +109,7 @@
private:
ClangTidyError &Error;
+ bool FixResolutions;
};
} // end anonymous namespace
@@ -264,10 +270,10 @@
}
ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
- ClangTidyContext &Ctx, bool RemoveIncompatibleErrors)
+ ClangTidyContext &Ctx, bool RemoveIncompatibleErrors , bool FixResolutions )
: Context(Ctx), RemoveIncompatibleErrors(RemoveIncompatibleErrors),
- LastErrorRelatesToUserCode(false), LastErrorPassesLineFilter(false),
- LastErrorWasIgnored(false) {}
+ FixResolutions(FixResolutions), LastErrorRelatesToUserCode(false),
+ LastErrorPassesLineFilter(false), LastErrorWasIgnored(false) {}
void ClangTidyDiagnosticConsumer::finalizeLastError() {
if (!Errors.empty()) {
@@ -444,7 +450,7 @@
ClangTidyDiagnosticRenderer Converter(
Context.getLangOpts(), &Context.DiagEngine->getDiagnosticOptions(),
- Errors.back());
+ Errors.back(), FixResolutions);
SmallString<100> Message;
Info.FormatDiagnostic(Message);
FullSourceLoc Loc;
Index: tools/extra/clang-tidy/ClangTidy.h
===================================================================
--- tools/extra/clang-tidy/ClangTidy.h
+++ tools/extra/clang-tidy/ClangTidy.h
@@ -235,7 +235,7 @@
const tooling::CompilationDatabase &Compilations,
ArrayRef<std::string> InputFiles,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS,
- bool EnableCheckProfile = false,
+ bool FixResolutions, bool EnableCheckProfile = false,
llvm::StringRef StoreCheckProfile = StringRef());
// FIXME: This interface will need to be significantly extended to be useful.
Index: tools/extra/clang-tidy/ClangTidy.cpp
===================================================================
--- tools/extra/clang-tidy/ClangTidy.cpp
+++ tools/extra/clang-tidy/ClangTidy.cpp
@@ -507,7 +507,8 @@
const CompilationDatabase &Compilations,
ArrayRef<std::string> InputFiles,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS,
- bool EnableCheckProfile, llvm::StringRef StoreCheckProfile) {
+ bool FixResolutions, bool EnableCheckProfile,
+ llvm::StringRef StoreCheckProfile) {
ClangTool Tool(Compilations, InputFiles,
std::make_shared<PCHContainerOperations>(), BaseFS);
@@ -550,7 +551,7 @@
Context.setEnableProfiling(EnableCheckProfile);
Context.setProfileStoragePrefix(StoreCheckProfile);
- ClangTidyDiagnosticConsumer DiagConsumer(Context);
+ ClangTidyDiagnosticConsumer DiagConsumer(Context, true, FixResolutions);
DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions(),
&DiagConsumer, /*ShouldOwnClient=*/false);
Context.setDiagnosticsEngine(&DE);
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -649,6 +649,12 @@
(void) P;
assert(N->pred_size() == 1);
N = *N->pred_begin();
+ if (N->pred_size() != 1) {
+ // FIXME: The CFG-based reasoning below does not apply.
+ // Do not set the value for the expression. It'd be UnknownVal by default.
+ Bldr.generateNode(B, Pred, state);
+ return;
+ }
}
assert(N->pred_size() == 1);
N = *N->pred_begin();
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
