[Custom-DefinesAndIncludes]: Objective-C++ support
ClosedPublic

Authored by rjvbb on Sep 26 2018, 10:37 AM.

Details

Summary

This introduces support for ObjC++ in the custom-definesandincludes plugin.
See also D15530

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 3275
Build 3293: arc lint + arc unit
rjvbb requested review of this revision.Sep 26 2018, 10:37 AM
rjvbb created this revision.
rjvbb updated this revision to Diff 42359.Sep 26 2018, 10:40 AM

Removed left-over x-objchdr reference (sorry about that).

rjvbb set the repository for this revision to R32 KDevelop.Sep 26 2018, 10:40 AM

So I think there are two questions to decide here:

  • Do we want a -std= flag, does Clang even accept that?
  • Should we just take the ObjC flags from the C flags, and ObjC++ flags from the C++ flags, or should they be set independently?
plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.cpp
68–80

Does ObjC++ not have a -std= flag?

plugins/custom-definesandincludes/compilerprovider/settingsmanager.cpp
74–75

Do you want to be able to define custom parser arguments? As above, are there multiple standards or language versions of ObjC++? I'd guess so if it is based on C++.

108

Since we didn't copy the C string for ObjC, we should similarly not tie Cpp and ObjCpp together. Unless there is a good reason to do that. But then we should do the same for C and ObjC.

plugins/custom-definesandincludes/kcm_widget/parserwidget.cpp
61

So here you say c++11.

rjvbb marked 4 inline comments as done.Sep 26 2018, 3:17 PM
  • Do we want a -std= flag, does Clang even accept that?

Yes, clang accepts it, as long as you don't ask for an impossible combination (C or ObjC with std=c++11 for instance).

I tried not adding the flag explicitly, but then it is added by languageStandard().

  • Should we just take the ObjC flags from the C flags, and ObjC++ flags from the C++ flags, or should they be set independently?

I guess I haven't yet taken a clear decision on that? That had to do with trying not to add a standards definition which seemed like a good idea because I haven't found a clear statement about the C standard that ObjC 2.0 is based on. But that question is probably moot because from what I understand the ObjC standard just describes the superset. I came across a post stating that using Objective-C code in a C11 (C++11) file gives you ObjC11 (ObjC++11).

plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.cpp
68–80

Why do you ask, it's getting -std=c++11 here (if no -std flag was given yet)?

plugins/custom-definesandincludes/compilerprovider/settingsmanager.cpp
74–75

From what I've seen there are basically as many ObjC(++) standards as there are C(++) standards.

There are a few ObjC-specific compiler options (which are illegal for C/C++), for instance concerning ObjC exceptions. I'm not certain they change anything for the parser though (esp. not the parser in its current form).

So I'm tempted to say that for now we could and should simply use the C/C++ options for ObjC/ObjC++ and re-evaluate if and when the need for specific options arises.

plugins/custom-definesandincludes/kcm_widget/parserwidget.cpp
61

Yes, but "there" too (if not I'm already in need of another vacation ^^)

rjvbb updated this revision to Diff 42377.Sep 26 2018, 3:36 PM
rjvbb marked 3 inline comments as done.

reuse the C(++) parser options for ObjC(++).

rjvbb set the repository for this revision to R32 KDevelop.Sep 26 2018, 3:36 PM
aaronpuchert added inline comments.Sep 26 2018, 4:16 PM
plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.cpp
68–80

Now that we pass in the language type, maybe we should handle all cases. So for C and ObjC we don't want to return an empty string, but (say) -std=c99. Maybe we can make this match the default arguments in plugins/custom-definesandincludes/compilerprovider/settingsmanager.cpp.

plugins/custom-definesandincludes/compilerprovider/settingsmanager.cpp
74–75

Ok, I think that's fine for now.

rjvbb marked 2 inline comments as done.Sep 26 2018, 5:22 PM
rjvbb added inline comments.
plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.cpp
68–80

Done. You know I dislike multiple exit points but for you I avoided a temp. var and as many kinda superfluous breaks ;)

rjvbb updated this revision to Diff 42380.Sep 26 2018, 5:23 PM
rjvbb marked an inline comment as done.

Updated as suggested: handle all known language types in languageStandard()

rjvbb set the repository for this revision to R32 KDevelop.Sep 26 2018, 5:23 PM
aaronpuchert accepted this revision.Sep 26 2018, 11:43 PM

I didn't test it, but it looks fine to me.

plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.cpp
79 ↗(On Diff #42380)

I think this could be Q_UNREACHABLE().

This revision is now accepted and ready to land.Sep 26 2018, 11:43 PM
rjvbb added a comment.Sep 27 2018, 9:15 AM

I think this could be Q_UNREACHABLE().

What happens if you do get there? I'd hate to introduce something that causes an abort on a user's system because an unforeseen situation that slipped through "QC".

rjvbb added a comment.Sep 27 2018, 2:26 PM

__builtin_unreachable() does have strange effects:

#include <stdio.h>

int foo(int doit)
{
     switch(doit) {
          case 0:
          case 1:
          case 2:
               return doit;
          default:
               __builtin_unreachable();
     }
}

int main(int argc, char *argv[])
{
     fprintf(stderr, "foo(0)=%d\n", foo(0));
     foo(1);
     foo(2);
     fprintf(stderr, "unreachable foo(4)=");
     fprintf(stderr, "%d\n", foo(4));
     return 0;
}

Depending on how and with what compiler I build this, the program either loops (from the foo(4) call back to the start of main) until it SEGVs, or it terminates after printing foo(4)=4

IOW, it looks like it just causes UB in release builds when the assert is removed from Q_UNREACHABLE :-/

Given that languageOption also has Q_UNREACHABLE(), and both are always called together, any crash that could happen would have already happened before. The function is called from CompilerProvider::{defines,includes} and both catch the Other case before calling into the ICompiler implementation. Maybe we should document the constraint (type != Utils::Other) in the ICompiler interface, but we should do it in a separate commit.

rjvbb updated this revision to Diff 42446.Sep 27 2018, 5:40 PM

Using Q_UNREACHABLE

rjvbb set the repository for this revision to R32 KDevelop.Sep 27 2018, 5:41 PM
rjvbb marked an inline comment as done.
aaronpuchert accepted this revision.Sep 27 2018, 5:46 PM
This revision was automatically updated to reflect the committed changes.