Skip to content

Commit 5d527e7

Browse files
ludviggunneludviggunne
committed
fix
1 parent b9d85e1 commit 5d527e7

5 files changed

Lines changed: 154 additions & 30 deletions

File tree

‎cli/cppcheckexecutor.cpp‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -661,13 +661,12 @@ void StdLogger::reportErr(const ErrorMessage &msg)
661661
msgCopy.classification = getClassification(msgCopy.guideline, mSettings.reportType);
662662

663663
// TODO: there should be no need for verbose and default messages here
664-
// Don't perform redundant reads for these formats, the code is not needed
665-
// for deduplication
666-
const bool noCode = mSettings.outputFormat == Settings::OutputFormat::xml ||
664+
const bool noContext = mSettings.outputFormat == Settings::OutputFormat::xml ||
667665
mSettings.outputFormat == Settings::OutputFormat::sarif;
666+
const ErrorMessage::SourceLineCallback callback = noContext ? nullptr : getSourceLineCallback();
668667
const std::string msgStr =
669668
msgCopy.toString(mSettings.verbose, mSettings.templateFormat,
670-
mSettings.templateLocation, noCode);
669+
mSettings.templateLocation, callback);
671670

672671
// Alert only about unique errors
673672
if (!mSettings.emitDuplicates && !mShownErrors.insert(msgStr).second)

‎lib/cppcheck.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,10 @@ class CppCheck::CppCheckLogger : public ErrorLogger
210210
}
211211

212212
// TODO: there should be no need for the verbose and default messages here
213-
// Code is not needed for deduplication
214-
const bool noCode = true;
215-
std::string errmsg = msg.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation, noCode);
213+
std::string errmsg = msg.toString(mSettings.verbose,
214+
mSettings.templateFormat,
215+
mSettings.templateLocation,
216+
nullptr);
216217
if (errmsg.empty())
217218
return;
218219

‎lib/errorlogger.cpp‎

Lines changed: 113 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -638,23 +638,6 @@ std::string ErrorMessage::toXML() const
638638
return printer.CStr();
639639
}
640640

641-
// TODO: read info from some shared resource instead?
642-
static std::string readCode(const std::string &file, int linenr, int column, const char endl[])
643-
{
644-
std::ifstream fin(file);
645-
std::string line;
646-
while (linenr > 0 && std::getline(fin,line)) {
647-
linenr--;
648-
}
649-
const std::string::size_type endPos = line.find_last_not_of("\r\n\t ");
650-
if (endPos + 1 < line.size())
651-
line.erase(endPos + 1);
652-
std::string::size_type pos = 0;
653-
while ((pos = line.find('\t', pos)) != std::string::npos)
654-
line[pos] = ' ';
655-
return line + endl + std::string((column>0 ? column-1 : 0), ' ') + '^';
656-
}
657-
658641
static void replaceSpecialChars(std::string& source)
659642
{
660643
// Support a few special characters to allow to specific formatting, see http://sourceforge.net/apps/phpbb/cppcheck/viewtopic.php?f=4&t=494&sid=21715d362c0dbafd3791da4d9522f814
@@ -729,7 +712,38 @@ static void replaceColors(std::string& source, bool erase) {
729712
replace(source, substitutionMapErase);
730713
}
731714

732-
std::string ErrorMessage::toString(bool verbose, const std::string &templateFormat, const std::string &templateLocation, bool noCode) const
715+
static std::string formatLine(std::string line, int column, const char endl[])
716+
{
717+
const std::string::size_type endPos = line.find_last_not_of("\r\n\t ");
718+
if (endPos + 1 < line.size())
719+
line.erase(endPos + 1);
720+
721+
std::string::size_type pos = 0;
722+
while ((pos = line.find('\t', pos)) != std::string::npos)
723+
line[pos] = ' ';
724+
725+
return line + endl + std::string((column>0 ? column-1 : 0), ' ') + '^';
726+
}
727+
728+
std::string ErrorMessage::directSourceLineCallback(const std::string &file,
729+
int linenr,
730+
int column,
731+
const char endl[],
732+
int cachePrio)
733+
{
734+
std::ifstream fin(file);
735+
std::string line;
736+
737+
while (linenr > 0 && std::getline(fin, line))
738+
--linenr;
739+
740+
return formatLine(line, column, endl);
741+
}
742+
743+
std::string ErrorMessage::toString(bool verbose,
744+
const std::string &templateFormat,
745+
const std::string &templateLocation,
746+
SourceLineCallback sourceLineCallback) const
733747
{
734748
assert(!templateFormat.empty());
735749

@@ -770,7 +784,12 @@ std::string ErrorMessage::toString(bool verbose, const std::string &templateForm
770784
endl = "\r\n";
771785
else
772786
endl = "\r";
773-
const std::string code = noCode ? "" : readCode(callStack.back().getOrigFile(), callStack.back().line, callStack.back().column, endl);
787+
const std::string code = sourceLineCallback == nullptr ?
788+
"" : sourceLineCallback(callStack.back().getOrigFile(),
789+
callStack.back().line,
790+
callStack.back().column,
791+
endl,
792+
0);
774793
findAndReplace(result, "{code}", code);
775794
}
776795
} else {
@@ -785,6 +804,7 @@ std::string ErrorMessage::toString(bool verbose, const std::string &templateForm
785804
replace(result, callStackSubstitutionMap);
786805
}
787806

807+
int cachePrio = -1;
788808
if (!templateLocation.empty() && callStack.size() >= 2U) {
789809
for (const FileLocation &fileLocation : callStack) {
790810
std::string text = templateLocation;
@@ -802,7 +822,12 @@ std::string ErrorMessage::toString(bool verbose, const std::string &templateForm
802822
endl = "\r\n";
803823
else
804824
endl = "\r";
805-
const std::string code = noCode ? "" : readCode(fileLocation.getOrigFile(), fileLocation.line, fileLocation.column, endl);
825+
const std::string code = sourceLineCallback == nullptr ?
826+
"" : sourceLineCallback(fileLocation.getOrigFile(),
827+
fileLocation.line,
828+
fileLocation.column,
829+
endl,
830+
cachePrio--);
806831
findAndReplace(text, "{code}", code);
807832
}
808833
result += '\n' + text;
@@ -1261,3 +1286,71 @@ std::map<std::string, std::string> createGuidelineMapping(ReportType reportType)
12611286

12621287
return guidelineMapping;
12631288
}
1289+
1290+
ErrorLogger::SourceCacheEntry::SourceCacheEntry(const std::string &file, int prio)
1291+
: prio(prio)
1292+
, file(file)
1293+
, stream(std::ifstream(file))
1294+
{
1295+
}
1296+
1297+
std::string ErrorLogger::sourceLineCallback(const std::string &file,
1298+
int linenr,
1299+
int column,
1300+
const char endl[],
1301+
int cachePrio)
1302+
{
1303+
// For sorting cache entries by priority
1304+
const auto heapCompare = [](const std::shared_ptr<SourceCacheEntry> &lhs, const std::shared_ptr<SourceCacheEntry> &rhs) {
1305+
return lhs->prio > rhs->prio;
1306+
};
1307+
1308+
std::shared_ptr<SourceCacheEntry> entry = nullptr;
1309+
1310+
const auto existing = std::find_if(
1311+
mSourceCache.begin(),
1312+
mSourceCache.end(),
1313+
[&] (const std::shared_ptr<SourceCacheEntry> &e) { return e->file == file; }
1314+
);
1315+
1316+
if (existing == mSourceCache.end()) {
1317+
if (mSourceCache.size() == mSourceCacheSize) {
1318+
// Evict the cache entry with lowest priority
1319+
std::pop_heap(mSourceCache.begin(), mSourceCache.end(), heapCompare);
1320+
mSourceCache.pop_back();
1321+
}
1322+
1323+
// Insert new entry
1324+
entry = std::make_shared<SourceCacheEntry>(file, cachePrio);
1325+
mSourceCache.push_back(entry);
1326+
std::push_heap(mSourceCache.begin(), mSourceCache.end(), heapCompare);
1327+
} else {
1328+
entry = *existing;
1329+
if (entry->prio < cachePrio) {
1330+
// Update priority and sort cache
1331+
entry->prio = cachePrio;
1332+
std::make_heap(mSourceCache.begin(), mSourceCache.end(), heapCompare);
1333+
}
1334+
}
1335+
1336+
entry->stream.clear();
1337+
entry->stream.seekg(0);
1338+
1339+
std::string line;
1340+
while (linenr > 0 && std::getline(entry->stream, line))
1341+
linenr--;
1342+
1343+
return formatLine(line, column, endl);
1344+
}
1345+
1346+
ErrorMessage::SourceLineCallback ErrorLogger::getSourceLineCallback()
1347+
{
1348+
return [this](const std::string &file,
1349+
int linenr,
1350+
int column,
1351+
const char endl[],
1352+
int cachePrio)
1353+
{
1354+
return sourceLineCallback(file, linenr, column, endl, cachePrio);
1355+
};
1356+
}

‎lib/errorlogger.h‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,16 @@
2626

2727
#include <cstdint>
2828
#include <ctime>
29+
#include <fstream>
2930
#include <list>
3031
#include <set>
3132
#include <sstream>
3233
#include <string>
3334
#include <utility>
3435
#include <vector>
3536
#include <map>
37+
#include <functional>
38+
#include <memory>
3639

3740
class Token;
3841
class TokenList;
@@ -102,6 +105,19 @@ class CPPCHECKLIB ErrorMessage {
102105
std::string mInfo;
103106
};
104107

108+
using SourceLineCallback = std::function<std::string (
109+
const std::string &file,
110+
int linenr,
111+
int column,
112+
const char endl[],
113+
int cachePrio)>;
114+
115+
static std::string directSourceLineCallback(const std::string &file,
116+
int linenr,
117+
int column,
118+
const char endl[],
119+
int cachePrio);
120+
105121
ErrorMessage(std::list<FileLocation> callStack,
106122
std::string file1,
107123
Severity severity,
@@ -152,13 +168,13 @@ class CPPCHECKLIB ErrorMessage {
152168
* or template to be used. E.g. "{file}:{line},{severity},{id},{message}"
153169
* @param templateLocation Format Empty string to use default output format
154170
* or template to be used. E.g. "{file}:{line},{info}"
155-
* @param noCode Always replace {code} with an empty string
171+
* @param sourceLineCallback Function used for fetching a line of source code for the error context
156172
* @return formatted string
157173
*/
158174
std::string toString(bool verbose,
159175
const std::string &templateFormat,
160176
const std::string &templateLocation,
161-
bool noCode = false) const;
177+
SourceLineCallback sourceLineCallback = directSourceLineCallback) const;
162178

163179
std::string serialize() const;
164180
/**
@@ -302,8 +318,23 @@ class CPPCHECKLIB ErrorLogger {
302318
return mCriticalErrorIds.count(id) != 0;
303319
}
304320

321+
ErrorMessage::SourceLineCallback getSourceLineCallback();
322+
305323
private:
306324
static const std::set<std::string> mCriticalErrorIds;
325+
static const std::size_t mSourceCacheSize = 4;
326+
327+
struct SourceCacheEntry {
328+
explicit SourceCacheEntry(const std::string &file, int prio);
329+
330+
int prio;
331+
std::string file;
332+
std::ifstream stream;
333+
};
334+
335+
std::vector<std::shared_ptr<SourceCacheEntry>> mSourceCache;
336+
337+
std::string sourceLineCallback(const std::string &file, int linenr, int column, const char endl[], int cachePrio);
307338
};
308339

309340
/// RAII class for reporting progress messages

‎test/testerrorlogger.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ class TestErrorLogger : public TestFixture {
472472
ASSERT_EQUALS(1, msg.callStack.size());
473473
const bool noCode = true;
474474
ASSERT_EQUALS("code.cpp:3:5: error: Programming error. [errorId]\n",
475-
msg.toString(false, "{file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]\n{code}", "", noCode));
475+
msg.toString(false, "{file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]\n{code}", "", nullptr));
476476
}
477477

478478
void CustomFormat() const {

0 commit comments

Comments
 (0)