Skip to content

Commit 5d9278e

Browse files
committed
Revert "[analyzer] Try to re-apply r283092 "Extend bug reports with extra notes"
Vector of smart pointers wasn't the thing that caused msvc crash. llvm-svn: 283537
1 parent fc36b58 commit 5d9278e

File tree

10 files changed

+58
-241
lines changed

10 files changed

+58
-241
lines changed

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -266,9 +266,6 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
266266
/// \sa shouldWidenLoops
267267
Optional<bool> WidenLoops;
268268

269-
/// \sa shouldDisplayNotesAsEvents
270-
Optional<bool> DisplayNotesAsEvents;
271-
272269
/// A helper function that retrieves option for a given full-qualified
273270
/// checker name.
274271
/// Options for checkers can be specified via 'analyzer-config' command-line
@@ -537,14 +534,6 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
537534
/// This is controlled by the 'widen-loops' config option.
538535
bool shouldWidenLoops();
539536

540-
/// Returns true if the bug reporter should transparently treat extra note
541-
/// diagnostic pieces as event diagnostic pieces. Useful when the diagnostic
542-
/// consumer doesn't support the extra note pieces.
543-
///
544-
/// This is controlled by the 'extra-notes-as-events' option, which defaults
545-
/// to false when unset.
546-
bool shouldDisplayNotesAsEvents();
547-
548537
public:
549538
AnalyzerOptions() :
550539
AnalysisStoreOpt(RegionStoreModel),

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

Lines changed: 2 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,6 @@ class BugReport : public llvm::ilist_node<BugReport> {
6767
typedef VisitorList::iterator visitor_iterator;
6868
typedef SmallVector<StringRef, 2> ExtraTextList;
6969

70-
// FIXME: We could have used a
71-
// SmallVector<llvm::IntrusiveRefCntPtr<PathDiagnosticNotePiece, 4>>
72-
// as NoteList, however MSVC 2013 crashes on reading this.
73-
typedef SmallVector<const PathDiagnosticNotePiece *, 4> NoteList;
74-
7570
protected:
7671
friend class BugReporter;
7772
friend class BugReportEquivClass;
@@ -87,8 +82,7 @@ class BugReport : public llvm::ilist_node<BugReport> {
8782
const ExplodedNode *ErrorNode;
8883
SmallVector<SourceRange, 4> Ranges;
8984
ExtraTextList ExtraText;
90-
NoteList Notes;
91-
85+
9286
typedef llvm::DenseSet<SymbolRef> Symbols;
9387
typedef llvm::DenseSet<const MemRegion *> Regions;
9488

@@ -183,18 +177,6 @@ class BugReport : public llvm::ilist_node<BugReport> {
183177
const BugType& getBugType() const { return BT; }
184178
BugType& getBugType() { return BT; }
185179

186-
/// \brief True when the report has an execution path associated with it.
187-
///
188-
/// A report is said to be path-sensitive if it was thrown against a
189-
/// particular exploded node in the path-sensitive analysis graph.
190-
/// Path-sensitive reports have their intermediate path diagnostics
191-
/// auto-generated, perhaps with the help of checker-defined visitors,
192-
/// and may contain extra notes.
193-
/// Path-insensitive reports consist only of a single warning message
194-
/// in a specific location, and perhaps extra notes.
195-
/// Path-sensitive checkers are allowed to throw path-insensitive reports.
196-
bool isPathSensitive() const { return ErrorNode != nullptr; }
197-
198180
const ExplodedNode *getErrorNode() const { return ErrorNode; }
199181

200182
StringRef getDescription() const { return Description; }
@@ -263,26 +245,7 @@ class BugReport : public llvm::ilist_node<BugReport> {
263245
void setDeclWithIssue(const Decl *declWithIssue) {
264246
DeclWithIssue = declWithIssue;
265247
}
266-
267-
/// Add new item to the list of additional notes that need to be attached to
268-
/// this path-insensitive report. If you want to add extra notes to a
269-
/// path-sensitive report, you need to use a BugReporterVisitor because it
270-
/// allows you to specify where exactly in the auto-generated path diagnostic
271-
/// the extra note should appear.
272-
void addNote(StringRef Msg, const PathDiagnosticLocation &Pos,
273-
ArrayRef<SourceRange> Ranges = {}) {
274-
auto *P = new PathDiagnosticNotePiece(Pos, Msg);
275-
276-
for (const auto &R : Ranges)
277-
P->addRange(R);
278-
279-
Notes.push_back(P);
280-
}
281-
282-
virtual const NoteList &getNotes() {
283-
return Notes;
284-
}
285-
248+
286249
/// \brief This allows for addition of meta data to the diagnostic.
287250
///
288251
/// Currently, only the HTMLDiagnosticClient knows how to display it.

clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ class PathDiagnosticLocationPair {
336336

337337
class PathDiagnosticPiece : public RefCountedBaseVPTR {
338338
public:
339-
enum Kind { ControlFlow, Event, Macro, Call, Note };
339+
enum Kind { ControlFlow, Event, Macro, Call };
340340
enum DisplayHint { Above, Below };
341341

342342
private:
@@ -452,8 +452,7 @@ class PathDiagnosticSpotPiece : public PathDiagnosticPiece {
452452
void Profile(llvm::FoldingSetNodeID &ID) const override;
453453

454454
static bool classof(const PathDiagnosticPiece *P) {
455-
return P->getKind() == Event || P->getKind() == Macro ||
456-
P->getKind() == Note;
455+
return P->getKind() == Event || P->getKind() == Macro;
457456
}
458457
};
459458

@@ -711,23 +710,6 @@ class PathDiagnosticMacroPiece : public PathDiagnosticSpotPiece {
711710
void Profile(llvm::FoldingSetNodeID &ID) const override;
712711
};
713712

714-
class PathDiagnosticNotePiece: public PathDiagnosticSpotPiece {
715-
public:
716-
PathDiagnosticNotePiece(const PathDiagnosticLocation &Pos, StringRef S,
717-
bool AddPosRange = true)
718-
: PathDiagnosticSpotPiece(Pos, S, Note, AddPosRange) {}
719-
720-
~PathDiagnosticNotePiece() override;
721-
722-
static inline bool classof(const PathDiagnosticPiece *P) {
723-
return P->getKind() == Note;
724-
}
725-
726-
void dump() const override;
727-
728-
void Profile(llvm::FoldingSetNodeID &ID) const override;
729-
};
730-
731713
/// PathDiagnostic - PathDiagnostic objects represent a single path-sensitive
732714
/// diagnostic. It represents an ordered-collection of PathDiagnosticPieces,
733715
/// each which represent the pieces of the path.

clang/lib/Rewrite/HTMLRewrite.cpp

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,6 @@ void html::AddHeaderFooterInternalBuiltinCSS(Rewriter &R, FileID FID,
324324
" .msgT { padding:0x; spacing:0x }\n"
325325
" .msgEvent { background-color:#fff8b4; color:#000000 }\n"
326326
" .msgControl { background-color:#bbbbbb; color:#000000 }\n"
327-
" .msgNote { background-color:#ddeeff; color:#000000 }\n"
328327
" .mrange { background-color:#dfddf3 }\n"
329328
" .mrange { border-bottom:1px solid #6F9DBE }\n"
330329
" .PathIndex { font-weight: bold; padding:0px 5px; "
@@ -344,12 +343,8 @@ void html::AddHeaderFooterInternalBuiltinCSS(Rewriter &R, FileID FID,
344343
" border-collapse: collapse; border-spacing: 0px;\n"
345344
" }\n"
346345
" td.rowname {\n"
347-
" text-align: right;\n"
348-
" vertical-align: top;\n"
349-
" font-weight: bold;\n"
350-
" color:#444444;\n"
351-
" padding-right:2ex;\n"
352-
" }\n"
346+
" text-align:right; font-weight:bold; color:#444444;\n"
347+
" padding-right:2ex; }\n"
353348
"</style>\n</head>\n<body>";
354349

355350
// Generate header

clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -344,10 +344,3 @@ bool AnalyzerOptions::shouldWidenLoops() {
344344
WidenLoops = getBooleanOption("widen-loops", /*Default=*/false);
345345
return WidenLoops.getValue();
346346
}
347-
348-
bool AnalyzerOptions::shouldDisplayNotesAsEvents() {
349-
if (!DisplayNotesAsEvents.hasValue())
350-
DisplayNotesAsEvents =
351-
getBooleanOption("notes-as-events", /*Default=*/false);
352-
return DisplayNotesAsEvents.getValue();
353-
}

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

Lines changed: 18 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -114,15 +114,15 @@ static void removeRedundantMsgs(PathPieces &path) {
114114
path.pop_front();
115115

116116
switch (piece->getKind()) {
117-
case PathDiagnosticPiece::Call:
117+
case clang::ento::PathDiagnosticPiece::Call:
118118
removeRedundantMsgs(cast<PathDiagnosticCallPiece>(piece)->path);
119119
break;
120-
case PathDiagnosticPiece::Macro:
120+
case clang::ento::PathDiagnosticPiece::Macro:
121121
removeRedundantMsgs(cast<PathDiagnosticMacroPiece>(piece)->subPieces);
122122
break;
123-
case PathDiagnosticPiece::ControlFlow:
123+
case clang::ento::PathDiagnosticPiece::ControlFlow:
124124
break;
125-
case PathDiagnosticPiece::Event: {
125+
case clang::ento::PathDiagnosticPiece::Event: {
126126
if (i == N-1)
127127
break;
128128

@@ -142,8 +142,6 @@ static void removeRedundantMsgs(PathPieces &path) {
142142
}
143143
break;
144144
}
145-
case PathDiagnosticPiece::Note:
146-
break;
147145
}
148146
path.push_back(piece);
149147
}
@@ -201,9 +199,6 @@ static bool removeUnneededCalls(PathPieces &pieces, BugReport *R,
201199
}
202200
case PathDiagnosticPiece::ControlFlow:
203201
break;
204-
205-
case PathDiagnosticPiece::Note:
206-
break;
207202
}
208203

209204
pieces.push_back(piece);
@@ -2559,12 +2554,6 @@ BugReport::~BugReport() {
25592554
while (!interestingSymbols.empty()) {
25602555
popInterestingSymbolsAndRegions();
25612556
}
2562-
2563-
// FIXME: Replace Notes with a list of shared pointers.
2564-
for (const auto *P: Notes) {
2565-
delete P;
2566-
}
2567-
Notes.clear();
25682557
}
25692558

25702559
const Decl *BugReport::getDeclWithIssue() const {
@@ -3416,28 +3405,25 @@ void BugReporter::FlushReport(BugReport *exampleReport,
34163405
exampleReport->getUniqueingLocation(),
34173406
exampleReport->getUniqueingDecl()));
34183407

3419-
if (exampleReport->isPathSensitive()) {
3420-
// Generate the full path diagnostic, using the generation scheme
3421-
// specified by the PathDiagnosticConsumer. Note that we have to generate
3422-
// path diagnostics even for consumers which do not support paths, because
3423-
// the BugReporterVisitors may mark this bug as a false positive.
3424-
assert(!bugReports.empty());
3425-
3426-
MaxBugClassSize =
3427-
std::max(bugReports.size(), static_cast<size_t>(MaxBugClassSize));
3408+
MaxBugClassSize = std::max(bugReports.size(),
3409+
static_cast<size_t>(MaxBugClassSize));
34283410

3411+
// Generate the full path diagnostic, using the generation scheme
3412+
// specified by the PathDiagnosticConsumer. Note that we have to generate
3413+
// path diagnostics even for consumers which do not support paths, because
3414+
// the BugReporterVisitors may mark this bug as a false positive.
3415+
if (!bugReports.empty())
34293416
if (!generatePathDiagnostic(*D.get(), PD, bugReports))
34303417
return;
34313418

3432-
MaxValidBugClassSize =
3433-
std::max(bugReports.size(), static_cast<size_t>(MaxValidBugClassSize));
3419+
MaxValidBugClassSize = std::max(bugReports.size(),
3420+
static_cast<size_t>(MaxValidBugClassSize));
34343421

3435-
// Examine the report and see if the last piece is in a header. Reset the
3436-
// report location to the last piece in the main source file.
3437-
AnalyzerOptions &Opts = getAnalyzerOptions();
3438-
if (Opts.shouldReportIssuesInMainSourceFile() && !Opts.AnalyzeAll)
3439-
D->resetDiagnosticLocationToMainFile();
3440-
}
3422+
// Examine the report and see if the last piece is in a header. Reset the
3423+
// report location to the last piece in the main source file.
3424+
AnalyzerOptions& Opts = getAnalyzerOptions();
3425+
if (Opts.shouldReportIssuesInMainSourceFile() && !Opts.AnalyzeAll)
3426+
D->resetDiagnosticLocationToMainFile();
34413427

34423428
// If the path is empty, generate a single step path with the location
34433429
// of the issue.
@@ -3450,28 +3436,6 @@ void BugReporter::FlushReport(BugReport *exampleReport,
34503436
D->setEndOfPath(std::move(piece));
34513437
}
34523438

3453-
PathPieces &Pieces = D->getMutablePieces();
3454-
bool ShouldConvert = getAnalyzerOptions().shouldDisplayNotesAsEvents();
3455-
// For path diagnostic consumers that don't support extra notes,
3456-
// we may optionally convert those to path notes.
3457-
for (auto I = exampleReport->getNotes().rbegin(),
3458-
E = exampleReport->getNotes().rend(); I != E; ++I) {
3459-
const PathDiagnosticNotePiece *Piece = *I;
3460-
// FIXME: getNotes() was supposed to return a list of shared pointers,
3461-
// and then we wouldn't normally create a new piece here,
3462-
// unless ShouldConvert is set.
3463-
PathDiagnosticPiece *ConvertedPiece =
3464-
ShouldConvert
3465-
? static_cast<PathDiagnosticPiece *>(new PathDiagnosticEventPiece(
3466-
Piece->getLocation(), Piece->getString()))
3467-
: static_cast<PathDiagnosticPiece *>(new PathDiagnosticNotePiece(
3468-
Piece->getLocation(), Piece->getString()));
3469-
for (const auto &R : Piece->getRanges())
3470-
ConvertedPiece->addRange(R);
3471-
3472-
Pieces.push_front(ConvertedPiece);
3473-
}
3474-
34753439
// Get the meta data.
34763440
const BugReport::ExtraTextList &Meta = exampleReport->getExtraText();
34773441
for (BugReport::ExtraTextList::const_iterator i = Meta.begin(),
@@ -3556,13 +3520,6 @@ LLVM_DUMP_METHOD void PathDiagnosticMacroPiece::dump() const {
35563520
// FIXME: Print which macro is being invoked.
35573521
}
35583522

3559-
LLVM_DUMP_METHOD void PathDiagnosticNotePiece::dump() const {
3560-
llvm::errs() << "NOTE\n--------------\n";
3561-
llvm::errs() << getString() << "\n";
3562-
llvm::errs() << " ---- at ----\n";
3563-
getLocation().dump();
3564-
}
3565-
35663523
LLVM_DUMP_METHOD void PathDiagnosticLocation::dump() const {
35673524
if (!isValid()) {
35683525
llvm::errs() << "<INVALID>\n";

0 commit comments

Comments
 (0)