This is an archive of the discontinued LLVM Phabricator instance.

Make Lit tests C++11 compatible - Objective-C++
ClosedPublic

Authored by tigerleapgorge on Feb 8 2017, 5:01 PM.

Details

Summary

I am continuing to make our Lit tests C++11 compatible.
This patch defaults five Objective-C++ tests to run under -std=gnu++98

This patch is essentially a continuation of Bug 24344
https://llvm.org/bugs/show_bug.cgi?id=24344
Fix r289167 resolved all tests inside the Rewriter directory,
This patch resolves the remaining tests outside the Rewriter directory.

Diff Detail

Event Timeline

tigerleapgorge created this revision.Feb 8 2017, 5:01 PM

These are all Objective-C++ tests, and AFAIK we don't intend to change the default Objective-C++ dialect when we finally do change the default C++ dialect.
So I think these tests do not need to be modified.

tigerleapgorge abandoned this revision.Feb 9 2017, 6:30 PM

These tests were failing because I accidentally changed the Objective-C++.
Abandoning this patch.

rjmccall edited edge metadata.Feb 9 2017, 9:46 PM

Hmm. Was there discussion about that? I see no reason not to bump ObjC++.

Hi John,

Here is the most recent discussion I can find on cfe-dev.
“I'm guessing that Objective-C/C++ is kind of passe, so nobody is really interested in modernizing it”
http://lists.llvm.org/pipermail/cfe-dev/2016-December/051844.html

As far as I am aware, there appears to be no strong reason to bump or not to bump ObjC++.

Cheers
Charles Li

Hi John,

Here is the most recent discussion I can find on cfe-dev.
“I'm guessing that Objective-C/C++ is kind of passe, so nobody is really interested in modernizing it”
http://lists.llvm.org/pipermail/cfe-dev/2016-December/051844.html

As far as I am aware, there appears to be no strong reason to bump or not to bump ObjC++.

It certainly simplifies the message to say that we've changed the default C++ dialect to C++11 across the board. That should apply to ObjC++ as well. I would not describe ObjC++ as passé; it's a very important language for Apple developers.

It is likely that the Rewriter generates C++98-only code. I believe the Rewriter is no longer being actively maintained; I'm not sure we're ready to propose removing it yet, but if there are specific problems with those tests, I think it makes some sense to just pass -std=gnu++98 for them.

John.

Hi John,

Here is the most recent discussion I can find on cfe-dev.
“I'm guessing that Objective-C/C++ is kind of passe, so nobody is really interested in modernizing it”
http://lists.llvm.org/pipermail/cfe-dev/2016-December/051844.html

As far as I am aware, there appears to be no strong reason to bump or not to bump ObjC++.

It certainly simplifies the message to say that we've changed the default C++ dialect to C++11 across the board. That should apply to ObjC++ as well. I would not describe ObjC++ as passé; it's a very important language for Apple developers.

Nice to know, although nobody piped up on the earlier cited discussion.

Sony is invested in making the lit tests C++11 clean so that we can upstream a change to make it the default C++ dialect for PS4. That will ensure that any new C++ tests are C++11 clean. This is one step in the direction of making C++11 (or even something newer) the default dialect for everybody.
However we are not an Objective-C++ vendor. We are neutral about changing the default dialect there; if you want to change the default dialect for Objective-C++, that's fine with us, but I don't think we can invest in learning enough about Objective-C++ to do the right thing with the existing Objective-C++ tests. In particular I don't know whether forcing 98 on these tests is the "right" solution; all we know is that it made the tests pass, which is not particularly surprising. I really think Apple would need to step up here if the default Objective-C++ dialect is going to change.

It is likely that the Rewriter generates C++98-only code. I believe the Rewriter is no longer being actively maintained; I'm not sure we're ready to propose removing it yet, but if there are specific problems with those tests, I think it makes some sense to just pass -std=gnu++98 for them.

John.

I am pretty sure we already modified the Rewriter tests to explicitly compile the rewritten C++ as -98 code. It's just the straight Objective-C++ tests where Charles has stepped back.

--paulr

Hi John,

Here is the most recent discussion I can find on cfe-dev.
“I'm guessing that Objective-C/C++ is kind of passe, so nobody is really interested in modernizing it”
http://lists.llvm.org/pipermail/cfe-dev/2016-December/051844.html

As far as I am aware, there appears to be no strong reason to bump or not to bump ObjC++.

It certainly simplifies the message to say that we've changed the default C++ dialect to C++11 across the board. That should apply to ObjC++ as well. I would not describe ObjC++ as passé; it's a very important language for Apple developers.

Nice to know, although nobody piped up on the earlier cited discussion.

Sony is invested in making the lit tests C++11 clean so that we can upstream a change to make it the default C++ dialect for PS4. That will ensure that any new C++ tests are C++11 clean. This is one step in the direction of making C++11 (or even something newer) the default dialect for everybody.
However we are not an Objective-C++ vendor. We are neutral about changing the default dialect there; if you want to change the default dialect for Objective-C++, that's fine with us, but I don't think we can invest in learning enough about Objective-C++ to do the right thing with the existing Objective-C++ tests. In particular I don't know whether forcing 98 on these tests is the "right" solution; all we know is that it made the tests pass, which is not particularly surprising.

I understand. That's part of why I do code review, because sometimes I can answer questions for other people. :) Forcing 98 on these tests is fine.

I really think Apple would need to step up here if the default Objective-C++ dialect is going to change.

I don't mind stepping up and doing this work. I just thought you'd already done it. This patch updates some tests; is that enough, or are there further changes required in order to change the default ObjC++ dialect?

John.

I really think Apple would need to step up here if the default Objective-C++ dialect is going to change.

I don't mind stepping up and doing this work. I just thought you'd already done it. This patch updates some tests; is that enough, or are there further changes required in order to change the default ObjC++ dialect?

John.

Charles tells me this is the last group of Objective-C++ tests that failed, so maybe there isn't anything else to do after all. I had thought there were more. He'll go ahead with this set.
We have another couple dozen C++ tests to straighten out, then we'll be ready to have the dev-list discussion about upgrading the default for 5.0, and whether it's the narrow case (just C++ and just for PS4, which was our original plan) or the big bang (both languages and for everybody).

I really think Apple would need to step up here if the default Objective-C++ dialect is going to change.

I don't mind stepping up and doing this work. I just thought you'd already done it. This patch updates some tests; is that enough, or are there further changes required in order to change the default ObjC++ dialect?

John.

Charles tells me this is the last group of Objective-C++ tests that failed, so maybe there isn't anything else to do after all. I had thought there were more. He'll go ahead with this set.
We have another couple dozen C++ tests to straighten out, then we'll be ready to have the dev-list discussion about upgrading the default for 5.0, and whether it's the narrow case (just C++ and just for PS4, which was our original plan) or the big bang (both languages and for everybody).

Okay, thanks, sounds good.

tigerleapgorge reclaimed this revision.Feb 13 2017, 3:09 PM

@rjmccall - Hi John, I have reopened this patch.

rjmccall accepted this revision.Feb 13 2017, 3:10 PM

LGTM.

This revision is now accepted and ready to land.Feb 13 2017, 3:10 PM
This revision was automatically updated to reflect the committed changes.