This is an archive of the discontinued LLVM Phabricator instance.

NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)
ClosedPublic

Authored by erik.pilkington on Oct 22 2018, 6:17 PM.

Diff Detail

Event Timeline

I'm very happy with reducing our configuration space like this, if the distinction is indeed unused (which it appears to be). (If compiling in ObjC1 mode actually is useful, we should instead have a command-line flag for that and tests; the fact that we don't tells us almost everything we need to know here.)

clang/include/clang/Basic/Features.def
95 ↗(On Diff #170548)

You touched this line last, so it's now your fault that there's missing space after the && :)

clang/lib/Basic/IdentifierTable.cpp
166–167 ↗(On Diff #170548)

Would it make sense to fold KEYOBJC and KEYARC together?

clang/lib/Sema/SemaCodeComplete.cpp
1365 ↗(On Diff #170548)

Space before ? please.

3335 ↗(On Diff #170548)

Space before ? please.

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
408 ↗(On Diff #170548)

Curious, this looks like it was the *only* way we previously ever got into ObjC1 mode. Any idea why this path turns on ObjC1 but not ObjC2?

erik.pilkington marked 4 inline comments as done.

Fix some spacing mistakes. Thanks!

erik.pilkington added inline comments.
clang/lib/Basic/IdentifierTable.cpp
166–167 ↗(On Diff #170548)

Yep, good point. Looks like we used to only enable these keywords in -fobjc-arc mode, but now that we're doing it for objective-c there isn't any distinction to be made here. I'll commit this in a follow-up.

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
408 ↗(On Diff #170548)

This comes from https://reviews.llvm.org/D11102. I can't really imagine this being anything but an oversight, I doubt that there is some subtle reason to use ObjC1. @jingham: Can you confirm that there isn't any reason to use just ObjC1 and not ObjC1+ObjC2 here?

theraven accepted this revision.Oct 23 2018, 11:13 AM
theraven added a subscriber: theraven.

Looks good for me, and removing all of the code describing Objective-C 4 as ObjC1 makes me happy.

clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
23 ↗(On Diff #170548)

LLVM style wants to get rid of the braces here (and the next few instances of this). Since you're touching this code, would you mind fixing that as well?

This revision is now accepted and ready to land.Oct 23 2018, 11:13 AM
This revision was automatically updated to reflect the committed changes.
clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
23 ↗(On Diff #170548)

Sure, I did this in the commit. Thanks for reviewing!

clang/lib/Basic/IdentifierTable.cpp
166–167 ↗(On Diff #170548)

Done in r345646.