| Summary: | C file parsed as a C++ file | ||
|---|---|---|---|
| Product: | [Applications] kdevelop | Reporter: | Pedro Ferreira <arkangath> |
| Component: | Language Support: CPP (Clang-based) | Assignee: | kdevelop-bugs-null |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | kalinichev.so.0 |
| Priority: | HI | Keywords: | release_blocker |
| Version: | git master | ||
| Target Milestone: | 5.0.0 | ||
| Platform: | Compiled Sources | ||
| OS: | Linux | ||
| Latest Commit: | http://commits.kde.org/kdevelop/70834035889654703d9b3eca2c4951a03a0eda53 | Version Fixed In: | |
| Sentry Crash Report: | |||
| Attachments: | Extension-based parsing | ||
Indeed. For the Clang-based version this is supposed to work out of the box. Also marking this as release blocker.
There are some serious issues with the "default compiler flags" provided by our Defines & Includes manager.
E.g.:
QString defaultArguments()
{
return {}; QStringLiteral("-ferror-limit=100 -fspell-checking -Wdocumentation -Wunused-parameter -Wunreachable-code -Wall -std=c++11");
}
^ Leaks into *every* compiler invocation afaics. Shouldn't append -std=c++11 for C files, though.
Whoops. s/{};// in above comment. (That was a debugging aid added by me)
Git commit e15e93ee159c460bb67800ba94eb36f3f118ec67 by Kevin Funk. Committed on 06/01/2016 at 13:40. Pushed by kfunk into branch '5.0'. Clang: Add test case for pure C file Expected fail atm, but test_files ignores it anyway. test_files does not check for problems in source files... A +3 -0 languages/clang/tests/files/purec.c [License: UNKNOWN] * The files marked with a * at the end have a non valid license. Please read: http://techbase.kde.org/Policies/Licensing_Policy and use the headers which are listed at that page. http://commits.kde.org/kdevelop/e15e93ee159c460bb67800ba94eb36f3f118ec67 Hi, We already had such discussion some time ago. Generally I think it's not a very good idea to determine language type solely on file extension (consider e.g. *.h - very popular extension used both for c and c++, and I see no reliable way how to distinguish it). Also you can already select language per project, directory or file: Project->Open configuration...->Language support->C/C++ parser. If anything we could use build manger to provide compiler invocation arguments, when available. > Leaks into *every* compiler invocation afaics. Shouldn't append -std=c++11 for C files, though. Could you be more specific, please? I don't really understand what is leaked and where... > Shouldn't append -std=c++11 for C files, though. Again, these arguments are fully configurable. @Sergey: I know it's fully configurable, but IMO we should at least make sure KDevelop treats .c files as C *without* having to configure anything. I agree that .h files are indistinguishable as such, but for implementation files it's different. Please see the purec.c file test I've added just recently. It currently is treated as C++ file and thus fails to compile. We have to make sure such a file is treated as a C file without further configuration, if not, this is a bug. Yes, we can definitely parse .c as plain C. But then again it solves nothing IMO, as people will come back complaining that headers are not parsed as plain C, but sources are. So, I don't know... (In reply to Sergey Kalinichev from comment #7) > Yes, we can definitely parse .c as plain C. But then again it solves nothing > IMO, as people will come back complaining that headers are not parsed as > plain C, but sources are. So, I don't know... True, but .h files are parsed as part of #include directives. Arguably, the clang parser shouldn't be called to parse those files independently. Without wanting to sound demeaning, other IDE's don't have this problem. How did they solve it? Setting the project settings won't work on the very common setting where a project has both C and C++ files (and potentially other languages). > but .h files are parsed as part of #include directives. When compiling yes, but inside IDE not necessarily (e.g. you can add a header file and work on it, and #include it later on). > Without wanting to sound demeaning, other IDE's don't have this problem. How did they solve it? I hardly doubt any other IDE performs such in-depth analysis as Clang does, so most likely "other IDE's" are simply not smart enough to correctly parse your test case. > Setting the project settings won't work on the very common setting where a project has both C and C++ files (and potentially other languages). Like I've already mentioned you can select language type per file basis. I'm with both of you here ;-) 1) Please make sure *.c (or anything else that has the C mimetype) is parsed as plain C. 2) Don't guess on headers, default to C++ there. For the common case they will be C++ compatible and if not, you often have a .c translation unit file including it and our code will then use that instead of the .h file anyways. Sergey, can you look into this issue? The following review request refers to this bug: https://git.reviewboard.kde.org/r/127027/ This needs to be fixed before a unit test can be added to the feature implemented by the review request. Git commit b930278bb44b37742bcbc19b12efeeb8026e1a3a by Kevin Funk, on behalf of Pedro Ferreira. Committed on 10/02/2016 at 16:21. Pushed by kfunk into branch '5.0'. Add support for CXType_FunctionNoProto Add support for CXType_FunctionNoProto as having the same behaviour as CXType_FunctionProto. Also, undefine a local macro. This doesn't fix any problems, but I believe its a Good Thing to clean up local macros when we're done with them. REVIEW: 127027 M +5 -3 languages/clang/duchain/builder.cpp http://commits.kde.org/kdevelop/b930278bb44b37742bcbc19b12efeeb8026e1a3a Ok, I'm going to look into it. Still I don't really see an easy way to integrate this feature in the UI. E.g. should it be something like adding 2 options: C++ profile: ... C profile: ... And then use given profile based on file extension. If so, what about headers? We will parse it as C++, but what if a project is in plain C? Yes we'll parse headers in plain C most of the time, because of TU pinning. But this solution is far from being perfect. Maybe we should add an option like: "parse headers in plain C"? And if this option is checked parse all .h files under given directory in plain C mode? Any suggestions on how to improve it? Created attachment 97352 [details]
Extension-based parsing
Hi Sergey,
I took a stab at fixing this one a while ago (patch attached) but did not have time to pursue. I might have tried to chew more than I could with this one anyway.
When I presented this a while ago on IRC, Milian suggested to use "c++" parsing for headers. I don't remember the full details of the explanation though.
Let me know if I can help though.
Git commit 70834035889654703d9b3eca2c4951a03a0eda53 by Sergey Kalinichev. Committed on 08/03/2016 at 06:05. Pushed by skalinichev into branch '5.0'. Parse C files in C mode Now there are 2 language profiles: one for C++, another one for C. The language type is determined by mime type. Since *.h files used in C and C++, by default they are parsed in C++ mode, to change that behavior there is a "Parse *.h headers in plain C" check-box. Related: bug 357774, bug 57156 Differential revision: https://phabricator.kde.org/D1047 M +1 -1 languages/clang/clangparsejob.cpp M +5 -0 languages/clang/clangsettings/clangsettingsmanager.cpp M +2 -0 languages/clang/clangsettings/clangsettingsmanager.h M +1 -1 languages/clang/duchain/parsesession.cpp M +2 -1 languages/clang/tests/test_duchain.cpp M +0 -1 languages/clang/tests/test_files.cpp M +12 -2 languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp M +64 -9 languages/plugins/custom-definesandincludes/compilerprovider/settingsmanager.cpp M +33 -2 languages/plugins/custom-definesandincludes/compilerprovider/settingsmanager.h M +15 -6 languages/plugins/custom-definesandincludes/definesandincludesmanager.cpp M +1 -0 languages/plugins/custom-definesandincludes/definesandincludesmanager.h M +2 -2 languages/plugins/custom-definesandincludes/idefinesandincludesmanager.h M +57 -24 languages/plugins/custom-definesandincludes/kcm_widget/parserwidget.cpp M +6 -3 languages/plugins/custom-definesandincludes/kcm_widget/parserwidget.h M +120 -8 languages/plugins/custom-definesandincludes/kcm_widget/parserwidget.ui M +4 -3 languages/plugins/custom-definesandincludes/kcm_widget/projectpathsmodel.cpp M +1 -1 languages/plugins/custom-definesandincludes/kcm_widget/projectpathsmodel.h M +3 -2 languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.cpp M +1 -1 languages/plugins/custom-definesandincludes/tests/test_definesandincludes.cpp http://commits.kde.org/kdevelop/70834035889654703d9b3eca2c4951a03a0eda53 |
The following simple valid C code yields errors on the parser: typedef enum { Zero, One } blaaa; static const blaaa b = 0 == 1; Since "blaa b" cannot be initialised with a "bool" constant. This is valid C, but not C++. The IDE should use the file extension to change the source language type. Reproducible: Always