Skip to content

Commit d5a054c

Browse files
firewavefirewave
authored
added Path::identify()/Path::isHeader2() and deprecated flawed Path::isCPP(), Path::isC() and Path::isHeader() (danmar#4681)
None of the `Path::is*()` function is working as expected or consistent (detailed via additional unit tests and deprecation comments). I tried to fix them but that would have made made cases with headers quite awkward to implement so I opted to generate a new function. This also optimized `Path::acceptFile()` a file. I am not 100% happy with including `settings.h` but I didn't see a place to move this to. If danmar#3774 gets merged we might be able to move the `enum` into `keywords.h` (that could also be renamed). I kept the old function in case somebody else is using these (they are in the API after all) and will remove them after the next release.
1 parent 36319b3 commit d5a054c

13 files changed

+291
-38
lines changed

‎Makefile

+4-4
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ $(libcppdir)/tokenize.o: lib/tokenize.cpp externals/simplecpp/simplecpp.h lib/ad
467467
$(libcppdir)/symboldatabase.o: lib/symboldatabase.cpp lib/addoninfo.h lib/astutils.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/keywords.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h
468468
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/symboldatabase.cpp
469469

470-
$(libcppdir)/addoninfo.o: lib/addoninfo.cpp externals/picojson/picojson.h lib/addoninfo.h lib/config.h lib/json.h lib/path.h lib/utils.h
470+
$(libcppdir)/addoninfo.o: lib/addoninfo.cpp externals/picojson/picojson.h lib/addoninfo.h lib/config.h lib/json.h lib/path.h lib/standards.h lib/utils.h
471471
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/addoninfo.cpp
472472

473473
$(libcppdir)/analyzerinfo.o: lib/analyzerinfo.cpp externals/tinyxml2/tinyxml2.h lib/analyzerinfo.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/path.h lib/platform.h lib/standards.h lib/utils.h lib/xml.h
@@ -599,13 +599,13 @@ $(libcppdir)/library.o: lib/library.cpp externals/tinyxml2/tinyxml2.h lib/astuti
599599
$(libcppdir)/mathlib.o: lib/mathlib.cpp externals/simplecpp/simplecpp.h lib/config.h lib/errortypes.h lib/mathlib.h lib/utils.h
600600
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/mathlib.cpp
601601

602-
$(libcppdir)/path.o: lib/path.cpp externals/simplecpp/simplecpp.h lib/config.h lib/path.h lib/utils.h
602+
$(libcppdir)/path.o: lib/path.cpp externals/simplecpp/simplecpp.h lib/config.h lib/path.h lib/standards.h lib/utils.h
603603
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/path.cpp
604604

605605
$(libcppdir)/pathanalysis.o: lib/pathanalysis.cpp lib/astutils.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/pathanalysis.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/vfvalue.h
606606
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/pathanalysis.cpp
607607

608-
$(libcppdir)/pathmatch.o: lib/pathmatch.cpp lib/config.h lib/path.h lib/pathmatch.h lib/utils.h
608+
$(libcppdir)/pathmatch.o: lib/pathmatch.cpp lib/config.h lib/path.h lib/pathmatch.h lib/standards.h lib/utils.h
609609
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/pathmatch.cpp
610610

611611
$(libcppdir)/platform.o: lib/platform.cpp externals/tinyxml2/tinyxml2.h lib/config.h lib/path.h lib/platform.h lib/standards.h lib/utils.h lib/xml.h
@@ -662,7 +662,7 @@ cli/cppcheckexecutorsig.o: cli/cppcheckexecutorsig.cpp cli/cppcheckexecutor.h cl
662662
cli/executor.o: cli/executor.cpp cli/executor.h lib/addoninfo.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
663663
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/executor.cpp
664664

665-
cli/filelister.o: cli/filelister.cpp cli/filelister.h lib/config.h lib/path.h lib/pathmatch.h lib/utils.h
665+
cli/filelister.o: cli/filelister.cpp cli/filelister.h lib/config.h lib/path.h lib/pathmatch.h lib/standards.h lib/utils.h
666666
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/filelister.cpp
667667

668668
cli/main.o: cli/main.cpp cli/cppcheckexecutor.h lib/config.h lib/errortypes.h lib/filesettings.h lib/platform.h lib/standards.h lib/utils.h

‎cli/cmdlineparser.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ bool CmdLineParser::fillSettingsFromArgs(int argc, const char* const argv[])
185185
// Output a warning for the user if he tries to exclude headers
186186
const std::vector<std::string>& ignored = getIgnoredPaths();
187187
const bool warn = std::any_of(ignored.cbegin(), ignored.cend(), [](const std::string& i) {
188-
return Path::isHeader(i);
188+
return Path::isHeader2(i);
189189
});
190190
if (warn) {
191191
mLogger.printMessage("filename exclusion does not apply to header (.h and .hpp) files.");

‎gui/resultstree.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -962,7 +962,7 @@ void ResultsTree::recheckSelectedFiles()
962962
askFileDir(currentFile);
963963
return;
964964
}
965-
if (Path::isHeader(currentFile.toStdString())) {
965+
if (Path::isHeader2(currentFile.toStdString())) {
966966
if (!data[FILE0].toString().isEmpty() && !selectedItems.contains(data[FILE0].toString())) {
967967
selectedItems<<((!mCheckPath.isEmpty() && (data[FILE0].toString().indexOf(mCheckPath) != 0)) ? (mCheckPath + "/" + data[FILE0].toString()) : data[FILE0].toString());
968968
if (!selectedItems.contains(fileNameWithCheckPath))

‎gui/resultsview.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ void ResultsView::updateDetails(const QModelIndex &index)
462462
QString formattedMsg = message;
463463

464464
const QString file0 = data["file0"].toString();
465-
if (!file0.isEmpty() && Path::isHeader(data["file"].toString().toStdString()))
465+
if (!file0.isEmpty() && Path::isHeader2(data["file"].toString().toStdString()))
466466
formattedMsg += QString("\n\n%1: %2").arg(tr("First included by"), QDir::toNativeSeparators(file0));
467467

468468
if (data["cwe"].toInt() > 0)

‎lib/config.h

+11
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,17 @@
110110
# define WARN_UNUSED
111111
#endif
112112

113+
// deprecated
114+
#if defined(__GNUC__) \
115+
|| defined(__clang__) \
116+
|| defined(__CPPCHECK__)
117+
# define DEPRECATED __attribute__((deprecated))
118+
#elif defined(_MSC_VER)
119+
# define DEPRECATED __declspec(deprecated)
120+
#else
121+
# define DEPRECATED
122+
#endif
123+
113124
#define REQUIRES(msg, ...) class=typename std::enable_if<__VA_ARGS__::value>::type
114125

115126
#include <string>

‎lib/cppcheck.cpp

+8-5
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,11 @@ static void createDumpFile(const Settings& settings,
195195
language = " language=\"cpp\"";
196196
break;
197197
case Standards::Language::None:
198-
if (Path::isCPP(filename))
198+
// TODO: error out on unknown language?
199+
const Standards::Language lang = Path::identify(filename);
200+
if (lang == Standards::Language::CPP)
199201
language = " language=\"cpp\"";
200-
else if (Path::isC(filename))
202+
else if (lang == Standards::Language::C)
201203
language = " language=\"c\"";
202204
break;
203205
}
@@ -429,7 +431,8 @@ unsigned int CppCheck::checkClang(const std::string &path)
429431
mErrorLogger.reportOut(std::string("Checking ") + path + " ...", Color::FgGreen);
430432

431433
// TODO: this ignores the configured language
432-
const std::string lang = Path::isCPP(path) ? "-x c++" : "-x c";
434+
const bool isCpp = Path::identify(path) == Standards::Language::CPP;
435+
const std::string langOpt = isCpp ? "-x c++" : "-x c";
433436
const std::string analyzerInfo = mSettings.buildDir.empty() ? std::string() : AnalyzerInformation::getAnalyzerInfoFile(mSettings.buildDir, path, emptyString);
434437
const std::string clangcmd = analyzerInfo + ".clang-cmd";
435438
const std::string clangStderr = analyzerInfo + ".clang-stderr";
@@ -442,9 +445,9 @@ unsigned int CppCheck::checkClang(const std::string &path)
442445
}
443446
#endif
444447

445-
std::string flags(lang + " ");
448+
std::string flags(langOpt + " ");
446449
// TODO: does not apply C standard
447-
if (Path::isCPP(path) && !mSettings.standards.stdValue.empty())
450+
if (isCpp && !mSettings.standards.stdValue.empty())
448451
flags += "-std=" + mSettings.standards.stdValue + " ";
449452

450453
for (const std::string &i: mSettings.includePaths)

‎lib/path.cpp

+52-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include <algorithm>
2727
#include <cstdlib>
2828
#include <sys/stat.h>
29+
#include <unordered_set>
2930
#include <utility>
3031

3132
#include <simplecpp.h>
@@ -193,6 +194,18 @@ std::string Path::getRelativePath(const std::string& absolutePath, const std::ve
193194
return absolutePath;
194195
}
195196

197+
static const std::unordered_set<std::string> cpp_src_exts = {
198+
".cpp", ".cxx", ".cc", ".c++", ".tpp", ".txx", ".ipp", ".ixx"
199+
};
200+
201+
static const std::unordered_set<std::string> c_src_exts = {
202+
".c", ".cl"
203+
};
204+
205+
static const std::unordered_set<std::string> header_exts = {
206+
".h", ".hpp", ".h++", ".hxx", ".hh"
207+
};
208+
196209
bool Path::isC(const std::string &path)
197210
{
198211
// In unix, ".C" is considered C++ file
@@ -220,15 +233,53 @@ bool Path::isCPP(const std::string &path)
220233

221234
bool Path::acceptFile(const std::string &path, const std::set<std::string> &extra)
222235
{
223-
return !Path::isHeader(path) && (Path::isCPP(path) || Path::isC(path) || extra.find(getFilenameExtension(path)) != extra.end());
236+
bool header = false;
237+
return (identify(path, &header) != Standards::Language::None && !header) || extra.find(getFilenameExtension(path)) != extra.end();
224238
}
225239

240+
// cppcheck-suppress unusedFunction
226241
bool Path::isHeader(const std::string &path)
227242
{
228243
const std::string extension = getFilenameExtensionInLowerCase(path);
229244
return startsWith(extension, ".h");
230245
}
231246

247+
Standards::Language Path::identify(const std::string &path, bool *header)
248+
{
249+
// cppcheck-suppress uninitvar - TODO: FP
250+
if (header)
251+
*header = false;
252+
253+
std::string ext = getFilenameExtension(path);
254+
if (ext == ".C")
255+
return Standards::Language::CPP;
256+
if (c_src_exts.find(ext) != c_src_exts.end())
257+
return Standards::Language::C;
258+
// cppcheck-suppress knownConditionTrueFalse - TODO: FP
259+
if (!caseInsensitiveFilesystem())
260+
strTolower(ext);
261+
if (ext == ".h") {
262+
if (header)
263+
*header = true;
264+
return Standards::Language::C; // treat as C for now
265+
}
266+
if (cpp_src_exts.find(ext) != cpp_src_exts.end())
267+
return Standards::Language::CPP;
268+
if (header_exts.find(ext) != header_exts.end()) {
269+
if (header)
270+
*header = true;
271+
return Standards::Language::CPP;
272+
}
273+
return Standards::Language::None;
274+
}
275+
276+
bool Path::isHeader2(const std::string &path)
277+
{
278+
bool header;
279+
(void)Path::identify(path, &header);
280+
return header;
281+
}
282+
232283
std::string Path::getAbsoluteFilePath(const std::string& filePath)
233284
{
234285
std::string absolute_path;

‎lib/path.h

+22-3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
//---------------------------------------------------------------------------
2323

2424
#include "config.h"
25+
#include "standards.h"
2526

2627
#include <set>
2728
#include <string>
@@ -156,22 +157,40 @@ class CPPCHECKLIB Path {
156157
* @brief Identify language based on file extension.
157158
* @param path filename to check. path info is optional
158159
* @return true if extension is meant for C files
160+
* @deprecated does not account for headers - use @identify() instead
159161
*/
160-
static bool isC(const std::string &path);
162+
static DEPRECATED bool isC(const std::string &path);
161163

162164
/**
163165
* @brief Identify language based on file extension.
164166
* @param path filename to check. path info is optional
165167
* @return true if extension is meant for C++ files
168+
* @deprecated returns true for some header extensions - use @identify() instead
166169
*/
167-
static bool isCPP(const std::string &path);
170+
static DEPRECATED bool isCPP(const std::string &path);
168171

169172
/**
170173
* @brief Is filename a header based on file extension
171174
* @param path filename to check. path info is optional
172175
* @return true if filename extension is meant for headers
176+
* @deprecated returns only heuristic result - use @identify() or @isHeader2() instead
173177
*/
174-
static bool isHeader(const std::string &path);
178+
static DEPRECATED bool isHeader(const std::string &path);
179+
180+
/**
181+
* @brief Is filename a header based on file extension
182+
* @param path filename to check. path info is optional
183+
* @return true if filename extension is meant for headers
184+
*/
185+
static bool isHeader2(const std::string &path);
186+
187+
/**
188+
* @brief Identify the language based on the file extension
189+
* @param path filename to check. path info is optional
190+
* @param header if provided indicates if the file is a header
191+
* @return the language type
192+
*/
193+
static Standards::Language identify(const std::string &path, bool *header = nullptr);
175194

176195
/**
177196
* @brief Get filename without a directory path part.

‎lib/preprocessor.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -730,11 +730,13 @@ static simplecpp::DUI createDUI(const Settings &mSettings, const std::string &cf
730730
dui.includePaths = mSettings.includePaths; // -I
731731
dui.includes = mSettings.userIncludes; // --include
732732
// TODO: use mSettings.standards.stdValue instead
733-
if (Path::isCPP(filename)) {
733+
// TODO: error out on unknown language?
734+
const Standards::Language lang = Path::identify(filename);
735+
if (lang == Standards::Language::CPP) {
734736
dui.std = mSettings.standards.getCPP();
735737
splitcfg(mSettings.platform.getLimitsDefines(Standards::getCPP(dui.std)), dui.defines, "");
736738
}
737-
else {
739+
else if (lang == Standards::Language::C) {
738740
dui.std = mSettings.standards.getC();
739741
splitcfg(mSettings.platform.getLimitsDefines(Standards::getC(dui.std)), dui.defines, "");
740742
}

‎lib/tokenize.cpp

+8-8
Original file line numberDiff line numberDiff line change
@@ -4156,7 +4156,7 @@ void VariableMap::addVariable(const std::string& varname, bool globalNamespace)
41564156
it->second = ++mVarId;
41574157
}
41584158

4159-
static bool setVarIdParseDeclaration(Token** tok, const VariableMap& variableMap, bool executableScope, bool cpp, bool c)
4159+
static bool setVarIdParseDeclaration(Token** tok, const VariableMap& variableMap, bool executableScope)
41604160
{
41614161
const Token* const tok1 = *tok;
41624162
Token* tok2 = *tok;
@@ -4174,14 +4174,14 @@ static bool setVarIdParseDeclaration(Token** tok, const VariableMap& variableMap
41744174
tok2 = tok2->linkAt(1)->next();
41754175
continue;
41764176
}
4177-
if (cpp && Token::Match(tok2, "namespace|public|private|protected"))
4177+
if (tok2->isCpp() && Token::Match(tok2, "namespace|public|private|protected"))
41784178
return false;
4179-
if (cpp && Token::simpleMatch(tok2, "decltype (")) {
4179+
if (tok2->isCpp() && Token::simpleMatch(tok2, "decltype (")) {
41804180
typeCount = 1;
41814181
tok2 = tok2->linkAt(1)->next();
41824182
continue;
41834183
}
4184-
if (Token::Match(tok2, "struct|union|enum") || (!c && Token::Match(tok2, "class|typename"))) {
4184+
if (Token::Match(tok2, "struct|union|enum") || (tok2->isCpp() && Token::Match(tok2, "class|typename"))) {
41854185
hasstruct = true;
41864186
typeCount = 0;
41874187
singleNameCount = 0;
@@ -4197,8 +4197,8 @@ static bool setVarIdParseDeclaration(Token** tok, const VariableMap& variableMap
41974197
++typeCount;
41984198
++singleNameCount;
41994199
}
4200-
} else if (!c && ((TemplateSimplifier::templateParameters(tok2) > 0) ||
4201-
Token::simpleMatch(tok2, "< >") /* Ticket #4764 */)) {
4200+
} else if (tok2->isCpp() && ((TemplateSimplifier::templateParameters(tok2) > 0) ||
4201+
Token::simpleMatch(tok2, "< >") /* Ticket #4764 */)) {
42024202
const Token *start = *tok;
42034203
if (Token::Match(start->previous(), "%or%|%oror%|&&|&|^|+|-|*|/"))
42044204
return false;
@@ -4263,7 +4263,7 @@ static bool setVarIdParseDeclaration(Token** tok, const VariableMap& variableMap
42634263
}
42644264
}
42654265

4266-
if (cpp && tok3 && Token::simpleMatch(tok3->previous(), "] (") &&
4266+
if (tok3 && tok3->isCpp() && Token::simpleMatch(tok3->previous(), "] (") &&
42674267
(Token::simpleMatch(tok3->link(), ") {") || Token::Match(tok3->link(), ") . %name%")))
42684268
isLambdaArg = true;
42694269
}
@@ -4683,7 +4683,7 @@ void Tokenizer::setVarIdPass1()
46834683
}
46844684

46854685
try { /* Ticket #8151 */
4686-
decl = setVarIdParseDeclaration(&tok2, variableMap, scopeStack.top().isExecutable, isCPP(), isC());
4686+
decl = setVarIdParseDeclaration(&tok2, variableMap, scopeStack.top().isExecutable);
46874687
} catch (const Token * errTok) {
46884688
syntaxError(errTok);
46894689
}

‎lib/tokenlist.cpp

+1-4
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,7 @@ void TokenList::determineCppC()
8585
{
8686
// only try to determine it if it wasn't enforced
8787
if (mLang == Standards::Language::None) {
88-
if (Path::isC(getSourceFilePath()))
89-
mLang = Standards::Language::C;
90-
else if (Path::isCPP(getSourceFilePath()))
91-
mLang = Standards::Language::CPP;
88+
mLang = Path::identify(getSourceFilePath());
9289
}
9390
}
9491

0 commit comments

Comments
 (0)