Skip to content

Commit c2a70ee

Browse files
firewavefirewave
authored
do not throw simplecpp::Output from Preprocessor::preprocess() (danmar#8001)
1 parent d6f30e4 commit c2a70ee

File tree

8 files changed

+118
-68
lines changed

8 files changed

+118
-68
lines changed

‎lib/cppcheck.cpp‎

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1142,11 +1142,31 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
11421142
try {
11431143
TokenList tokenlist{mSettings, file.lang()};
11441144

1145-
// Create tokens, skip rest of iteration if failed
1146-
Timer::run("Tokenizer::createTokens", mSettings.showtime, &s_timerResults, [&]() {
1147-
simplecpp::TokenList tokensP = preprocessor.preprocess(currentConfig, files, true);
1148-
tokenlist.createTokens(std::move(tokensP));
1149-
});
1145+
{
1146+
bool skipCfg = false;
1147+
// Create tokens, skip rest of iteration if failed
1148+
Timer::run("Tokenizer::createTokens", mSettings.showtime, &s_timerResults, [&]() {
1149+
simplecpp::OutputList outputList_cfg;
1150+
simplecpp::TokenList tokensP = preprocessor.preprocess(currentConfig, files, outputList_cfg);
1151+
const simplecpp::Output* o = preprocessor.handleErrors(outputList_cfg);
1152+
if (!o) {
1153+
tokenlist.createTokens(std::move(tokensP));
1154+
}
1155+
else {
1156+
// #error etc during preprocessing
1157+
configurationError.push_back((currentConfig.empty() ? "\'\'" : currentConfig) + " : [" + o->location.file() + ':' + std::to_string(o->location.line) + "] " + o->msg);
1158+
--checkCount; // don't count invalid configurations
1159+
1160+
if (!hasValidConfig && currCfg == *configurations.rbegin()) {
1161+
// If there is no valid configuration then report error..
1162+
preprocessor.error(o->location.file(), o->location.line, o->location.col, o->msg, o->type);
1163+
}
1164+
skipCfg = true;
1165+
}
1166+
});
1167+
if (skipCfg)
1168+
continue;
1169+
}
11501170
hasValidConfig = true;
11511171

11521172
Tokenizer tokenizer(std::move(tokenlist), mErrorLogger);
@@ -1215,17 +1235,6 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
12151235
ErrorMessage errmsg = ErrorMessage::fromInternalError(e, &tokenizer.list, file.spath());
12161236
mErrorLogger.reportErr(errmsg);
12171237
}
1218-
} catch (const simplecpp::Output &o) {
1219-
// #error etc during preprocessing
1220-
configurationError.push_back((currentConfig.empty() ? "\'\'" : currentConfig) + " : [" + o.location.file() + ':' + std::to_string(o.location.line) + "] " + o.msg);
1221-
--checkCount; // don't count invalid configurations
1222-
1223-
if (!hasValidConfig && currCfg == *configurations.rbegin()) {
1224-
// If there is no valid configuration then report error..
1225-
preprocessor.error(o.location.file(), o.location.line, o.location.col, o.msg, o.type);
1226-
}
1227-
continue;
1228-
12291238
} catch (const TerminateException &) {
12301239
// Analysis is terminated
12311240
if (analyzerInformation)

‎lib/cppcheck.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ class CPPCHECKLIB CppCheck {
203203
* @brief Check a file using stream
204204
* @param file the file
205205
* @param cfgname cfg name
206-
* @param createTokenList a function to create the simplecpp::TokenList with - throws simplecpp::Output
206+
* @param createTokenList a function to create the simplecpp::TokenList with
207207
* @return number of errors found
208208
*/
209209
unsigned int checkInternal(const FileWithDetails& file, const std::string &cfgname, int fileIndex, const CreateTokenListFn& createTokenList);

‎lib/preprocessor.cpp‎

Lines changed: 13 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -742,38 +742,10 @@ static simplecpp::DUI createDUI(const Settings &mSettings, const std::string &cf
742742
return dui;
743743
}
744744

745-
bool Preprocessor::hasErrors(const simplecpp::Output &output)
746-
{
747-
switch (output.type) {
748-
case simplecpp::Output::ERROR:
749-
case simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY:
750-
case simplecpp::Output::SYNTAX_ERROR:
751-
case simplecpp::Output::UNHANDLED_CHAR_ERROR:
752-
case simplecpp::Output::EXPLICIT_INCLUDE_NOT_FOUND:
753-
case simplecpp::Output::FILE_NOT_FOUND:
754-
case simplecpp::Output::DUI_ERROR:
755-
return true;
756-
case simplecpp::Output::WARNING:
757-
case simplecpp::Output::MISSING_HEADER:
758-
case simplecpp::Output::PORTABILITY_BACKSLASH:
759-
break;
760-
}
761-
return false;
762-
}
763-
764-
bool Preprocessor::handleErrors(const simplecpp::OutputList& outputList, bool throwError)
745+
const simplecpp::Output* Preprocessor::handleErrors(const simplecpp::OutputList& outputList)
765746
{
766747
const bool showerror = (!mSettings.userDefines.empty() && !mSettings.force);
767-
const bool hasError = reportOutput(outputList, showerror);
768-
if (throwError) {
769-
const auto it = std::find_if(outputList.cbegin(), outputList.cend(), [](const simplecpp::Output &output){
770-
return hasErrors(output);
771-
});
772-
if (it != outputList.cend()) {
773-
throw *it;
774-
}
775-
}
776-
return hasError;
748+
return reportOutput(outputList, showerror);
777749
}
778750

779751
bool Preprocessor::loadFiles(std::vector<std::string> &files)
@@ -782,7 +754,7 @@ bool Preprocessor::loadFiles(std::vector<std::string> &files)
782754

783755
simplecpp::OutputList outputList;
784756
mFileCache = simplecpp::load(mTokens, files, dui, &outputList);
785-
return !handleErrors(outputList, false);
757+
return !handleErrors(outputList);
786758
}
787759

788760
void Preprocessor::removeComments()
@@ -813,28 +785,27 @@ void Preprocessor::setPlatformInfo()
813785
mTokens.sizeOfType["long double *"] = mSettings.platform.sizeof_pointer;
814786
}
815787

816-
simplecpp::TokenList Preprocessor::preprocess(const std::string &cfg, std::vector<std::string> &files, bool throwError)
788+
simplecpp::TokenList Preprocessor::preprocess(const std::string &cfg, std::vector<std::string> &files, simplecpp::OutputList& outputList)
817789
{
818790
const simplecpp::DUI dui = createDUI(mSettings, cfg, mLang);
819791

820-
simplecpp::OutputList outputList;
821792
std::list<simplecpp::MacroUsage> macroUsage;
822793
std::list<simplecpp::IfCond> ifCond;
823794
simplecpp::TokenList tokens2(files);
824795
simplecpp::preprocess(tokens2, mTokens, files, mFileCache, dui, &outputList, &macroUsage, &ifCond);
825796
mMacroUsage = std::move(macroUsage);
826797
mIfCond = std::move(ifCond);
827798

828-
(void)handleErrors(outputList, throwError);
829-
830799
tokens2.removeComments();
831800

832801
return tokens2;
833802
}
834803

835804
std::string Preprocessor::getcode(const std::string &cfg, std::vector<std::string> &files, const bool writeLocations)
836805
{
837-
simplecpp::TokenList tokens2 = preprocess(cfg, files, false);
806+
simplecpp::OutputList outputList;
807+
simplecpp::TokenList tokens2 = preprocess(cfg, files, outputList);
808+
handleErrors(outputList);
838809
unsigned int prevfile = 0;
839810
unsigned int line = 1;
840811
std::ostringstream ret;
@@ -859,14 +830,14 @@ std::string Preprocessor::getcode(const std::string &cfg, std::vector<std::strin
859830
return ret.str();
860831
}
861832

862-
bool Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool showerror)
833+
const simplecpp::Output* Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool showerror)
863834
{
864-
bool hasError = false;
835+
const simplecpp::Output* out_ret = nullptr;
865836

866837
for (const simplecpp::Output &out : outputList) {
867838
switch (out.type) {
868839
case simplecpp::Output::ERROR:
869-
hasError = true;
840+
out_ret = &out;
870841
if (!startsWith(out.msg,"#error") || showerror)
871842
error(out.location.file(), out.location.line, out.location.col, out.msg, out.type);
872843
break;
@@ -884,19 +855,19 @@ bool Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool sh
884855
case simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY:
885856
case simplecpp::Output::SYNTAX_ERROR:
886857
case simplecpp::Output::UNHANDLED_CHAR_ERROR:
887-
hasError = true;
858+
out_ret = &out;
888859
error(out.location.file(), out.location.line, out.location.col, out.msg, out.type);
889860
break;
890861
case simplecpp::Output::EXPLICIT_INCLUDE_NOT_FOUND:
891862
case simplecpp::Output::FILE_NOT_FOUND:
892863
case simplecpp::Output::DUI_ERROR:
893-
hasError = true;
864+
out_ret = &out;
894865
error("", 0, 0, out.msg, out.type);
895866
break;
896867
}
897868
}
898869

899-
return hasError;
870+
return out_ret;
900871
}
901872

902873
static std::string simplecppErrToId(simplecpp::Output::Type type)

‎lib/preprocessor.h‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
118118

119119
void setPlatformInfo();
120120

121-
simplecpp::TokenList preprocess(const std::string &cfg, std::vector<std::string> &files, bool throwError = false);
121+
simplecpp::TokenList preprocess(const std::string &cfg, std::vector<std::string> &files, simplecpp::OutputList& outputList);
122122

123123
std::string getcode(const std::string &cfg, std::vector<std::string> &files, bool writeLocations);
124124

@@ -139,15 +139,13 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
139139
*/
140140
void dump(std::ostream &out) const;
141141

142-
bool reportOutput(const simplecpp::OutputList &outputList, bool showerror);
142+
const simplecpp::Output* reportOutput(const simplecpp::OutputList &outputList, bool showerror);
143143

144144
void error(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg, simplecpp::Output::Type type);
145145

146-
private:
147-
static bool hasErrors(const simplecpp::Output &output);
148-
149-
bool handleErrors(const simplecpp::OutputList &outputList, bool throwError);
146+
const simplecpp::Output* handleErrors(const simplecpp::OutputList &outputList);
150147

148+
private:
151149
static void simplifyPragmaAsmPrivate(simplecpp::TokenList &tokenList);
152150

153151
/**

‎test/cli/other_test.py‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3986,3 +3986,69 @@ def test_max_configs(tmp_path, max_configs, number_of_configs, check_config, exp
39863986
'{}:0:0: information: Too many #ifdef configurations - cppcheck only checks {} of {} configurations. Use --force to check all configurations. [toomanyconfigs]'
39873987
.format(test_file, max_configs, number_of_configs)
39883988
]
3989+
3990+
3991+
def test_no_valid_configuration(tmp_path):
3992+
test_file = tmp_path / 'test.c'
3993+
with open(test_file, "w") as f:
3994+
f.write(
3995+
"""#include ""
3996+
#ifdef DEF_1
3997+
#include ""
3998+
#endif
3999+
""")
4000+
4001+
args = [
4002+
'--template=simple',
4003+
'--emit-duplicates',
4004+
'--enable=information',
4005+
'--suppress=checkersReport',
4006+
str(test_file)
4007+
]
4008+
4009+
exitcode, stdout, stderr = cppcheck(args)
4010+
assert exitcode == 0, stdout
4011+
assert stdout.splitlines() == [
4012+
'Checking {} ...'.format(test_file)
4013+
]
4014+
# TODO: this lacks context about the configuration which encounters these errors
4015+
# TODO: add message when a configuration is dropped?
4016+
assert stderr.splitlines() == [
4017+
# TODO: should only report the error once
4018+
'{}:1:2: error: No header in #include [syntaxError]'.format(test_file),
4019+
'{}:1:2: error: No header in #include [syntaxError]'.format(test_file),
4020+
'{}:1:2: error: No header in #include [syntaxError]'.format(test_file),
4021+
'{}:0:0: information: This file is not analyzed. Cppcheck failed to extract a valid configuration. Use -v for more details. [noValidConfiguration]'.format(test_file)
4022+
]
4023+
4024+
4025+
def test_no_valid_configuration_check_config(tmp_path):
4026+
test_file = tmp_path / 'test.c'
4027+
with open(test_file, "w") as f:
4028+
f.write(
4029+
"""#include ""
4030+
#ifdef DEF_1
4031+
#include ""
4032+
#endif
4033+
""")
4034+
4035+
args = [
4036+
'--template=simple',
4037+
'--emit-duplicates',
4038+
'--enable=information',
4039+
'--suppress=checkersReport',
4040+
'--check-config',
4041+
str(test_file)
4042+
]
4043+
4044+
exitcode, stdout, stderr = cppcheck(args)
4045+
assert exitcode == 0, stdout
4046+
assert stdout.splitlines() == [
4047+
'Checking {} ...'.format(test_file)
4048+
]
4049+
# TODO: this lacks context about the configuration which encounters these errors
4050+
# TODO: add message when a configuration is dropped
4051+
assert stderr.splitlines() == [
4052+
'{}:1:2: error: No header in #include [syntaxError]'.format(test_file),
4053+
'{}:1:2: error: No header in #include [syntaxError]'.format(test_file)
4054+
]

‎test/helpers.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,9 @@ void SimpleTokenizer2::preprocess(const char* code, std::size_t size, std::vecto
116116
simplecpp::TokenList tokens1(code, size, files, file0);
117117

118118
Preprocessor preprocessor(tokens1, tokenizer.getSettings(), errorlogger, Path::identify(tokens1.getFiles()[0], false));
119-
simplecpp::TokenList tokens2 = preprocessor.preprocess("", files, true);
119+
simplecpp::OutputList outputList;
120+
simplecpp::TokenList tokens2 = preprocessor.preprocess("", files, outputList);
121+
(void)preprocessor.reportOutput(outputList, true);
120122

121123
// Tokenizer..
122124
tokenizer.list.createTokens(std::move(tokens2));

‎test/testpreprocessor.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,10 @@ class TestPreprocessor : public TestFixture {
5555
std::vector<std::string> files;
5656
simplecpp::TokenList tokens1 = simplecpp::TokenList(code, files, "file.cpp", &outputList);
5757
Preprocessor p(tokens1, settingsDefault, errorLogger, Path::identify(tokens1.getFiles()[0], false));
58-
simplecpp::TokenList tokens2 = p.preprocess("", files, true);
5958
(void)p.reportOutput(outputList, true);
59+
simplecpp::OutputList outputList_pp;
60+
simplecpp::TokenList tokens2 = p.preprocess("", files, outputList_pp);
61+
(void)p.reportOutput(outputList_pp, true);
6062
return tokens2.stringify();
6163
}
6264

‎test/testtokenlist.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,9 @@ class TestTokenList : public TestFixture {
158158
std::vector<std::string> files;
159159
simplecpp::TokenList tokens1(code, files, "poll.h", nullptr);
160160
Preprocessor preprocessor(tokens1, settingsDefault, *this, Path::identify(tokens1.getFiles()[0], false));
161-
simplecpp::TokenList tokensP = preprocessor.preprocess("", files, true);
161+
simplecpp::OutputList outputList_pp;
162+
simplecpp::TokenList tokensP = preprocessor.preprocess("", files, outputList_pp);
163+
ASSERT(!preprocessor.reportOutput(outputList_pp, true));
162164
TokenList tokenlist(settingsDefault, Standards::Language::C); // headers are treated as C files
163165
tokenlist.createTokens(std::move(tokensP)); // do not assert
164166
}

0 commit comments

Comments
 (0)