Bug 357615 - C file parsed as a C++ file
Summary: C file parsed as a C++ file
Status: RESOLVED FIXED
Alias: None
Product: kdevelop
Classification: Applications
Component: Language Support: CPP (Clang-based) (show other bugs)
Version: git master
Platform: Compiled Sources Linux
: HI normal
Target Milestone: 5.0.0
Assignee: kdevelop-bugs-null
URL:
Keywords: release_blocker
Depends on:
Blocks:
 
Reported: 2016-01-06 11:27 UTC by Pedro Ferreira
Modified: 2016-03-08 06:21 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Extension-based parsing (6.92 KB, patch)
2016-02-22 09:01 UTC, Pedro Ferreira
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Pedro Ferreira 2016-01-06 11:27:35 UTC
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
Comment 1 Kevin Funk 2016-01-06 11:49:58 UTC
Indeed. For the Clang-based version this is supposed to work out of the box.
Comment 2 Kevin Funk 2016-01-06 13:28:30 UTC
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.
Comment 3 Kevin Funk 2016-01-06 13:29:21 UTC
Whoops. s/{};// in above comment. (That was a debugging aid added by me)
Comment 4 Kevin Funk 2016-01-06 13:44:32 UTC
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
Comment 5 Sergey Kalinichev 2016-01-07 09:39:33 UTC
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.
Comment 6 Kevin Funk 2016-01-07 09:57:06 UTC
@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.
Comment 7 Sergey Kalinichev 2016-01-07 10:10:36 UTC
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...
Comment 8 Pedro Ferreira 2016-01-07 10:58:56 UTC
(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).
Comment 9 Sergey Kalinichev 2016-01-07 12:01:06 UTC
> 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.
Comment 10 Milian Wolff 2016-01-07 13:54:31 UTC
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.
Comment 11 Milian Wolff 2016-01-30 16:14:51 UTC
Sergey, can you look into this issue?
Comment 12 Pedro Ferreira 2016-02-10 14:21:43 UTC
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.
Comment 13 Kevin Funk 2016-02-10 16:22:08 UTC
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
Comment 14 Sergey Kalinichev 2016-02-22 07:35:38 UTC
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?
Comment 15 Pedro Ferreira 2016-02-22 09:01:05 UTC
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.
Comment 16 Sergey Kalinichev 2016-03-08 06:21:20 UTC
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