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

[erik.pilkington]

Summary

We haven't supported compiling ObjC1 for a long time (and never will again), so there isn't any reason to keep these separate. This patch replaces LangOpts::ObjC1 and LangOpts::ObjC2 with LangOpts::ObjC. See a recent thread on cfe-dev here: http://lists.llvm.org/pipermail/cfe-dev/2018-September/059468.html

Thanks for taking a look!
Erik

[rsmith]

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]

Fix some spacing mistakes. Thanks!

[erik.pilkington]

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]

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?

[erik.pilkington]

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.

clang-tidy/google/AvoidCStyleCastsCheck.cpp

if (SourceTypeAsWritten == DestTypeAsWritten) {
       diag(CastExpr->getBeginLoc(), "redundant cast to the same type")
           << FixItHint::CreateRemoval(ReplaceRange);
       return;
     }
   }
 
   // The rest of this check is only relevant to C++.
   // We also disable it for Objective-C++.
-  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC1 || getLangOpts().ObjC2)
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
     return;
   // Ignore code inside extern "C" {} blocks.
   if (!match(expr(hasAncestor(linkageSpecDecl())), *CastExpr, *Result.Context)
            .empty())
     return;
   // Ignore code in .c files and headers included from them, even if they are
   // compiled as C++.
   if (getCurrentMainFile().endswith(".c"))

clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp

 
 namespace clang {
 namespace tidy {
 namespace google {
 namespace objc {
 
 void AvoidThrowingObjCExceptionCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC)
     return;
-  }
 
   Finder->addMatcher(objcThrowStmt().bind("throwStmt"), this);
   Finder->addMatcher(
       objcMessageExpr(anyOf(hasSelector("raise:format:"),
                             hasSelector("raise:format:arguments:")),
                       hasReceiverType(asString("NSException")))
           .bind("raiseException"),
       this);

clang-tidy/google/GlobalVariableDeclarationCheck.cpp

FixItHint generateFixItHint(const VarDecl *Decl, bool IsConst) {
   return FixItHint::CreateReplacement(
       CharSourceRange::getTokenRange(SourceRange(Decl->getLocation())),
       llvm::StringRef(NewName));
 }
 }  // namespace
 
 void GlobalVariableDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // The relevant Style Guide rule only applies to Objective-C.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC)
     return;
-  }
+
   // need to add two matchers since we need to bind different ids to distinguish
   // constants and variables. Since bind() can only be called on node matchers,
   // we cannot make it in one matcher.
   //
   // Note that hasGlobalStorage() matches static variables declared locally
   // inside a function or method, so we need to exclude those with
   // isLocalVariable().
   Finder->addMatcher(

clang-tidy/objc/AvoidNSErrorInitCheck.cpp

 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace objc {
 
 void AvoidNSErrorInitCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC)
     return;
-  }
+
   Finder->addMatcher(objcMessageExpr(hasSelector("init"),
                                      hasReceiverType(asString("NSError *")))
                          .bind("nserrorInit"),
                      this);
 }
 
 void AvoidNSErrorInitCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *MatchedExpr =

clang-tidy/objc/ForbiddenSubclassingCheck.cpp

ForbiddenSubclassingCheck::ForbiddenSubclassingCheck(
     : ClangTidyCheck(Name, Context),
       ForbiddenSuperClassNames(
           utils::options::parseStringList(
               Options.get("ClassNames", DefaultForbiddenSuperClassNames))) {
 }
 
 void ForbiddenSubclassingCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC)
     return;
-  }
+
   Finder->addMatcher(
       objcInterfaceDecl(
           isSubclassOf(
               objcInterfaceDecl(
                   hasAnyName(
                       std::vector<StringRef>(
                           ForbiddenSuperClassNames.begin(),
                           ForbiddenSuperClassNames.end())))

clang-tidy/objc/PropertyDeclarationCheck.cpp

PropertyDeclarationCheck::PropertyDeclarationCheck(StringRef Name,
     : ClangTidyCheck(Name, Context),
       SpecialAcronyms(
           utils::options::parseStringList(Options.get("Acronyms", ""))),
       IncludeDefaultAcronyms(Options.get("IncludeDefaultAcronyms", true)),
       EscapedAcronyms() {}
 
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC)
     return;
-  }
+
   if (IncludeDefaultAcronyms) {
     EscapedAcronyms.reserve(llvm::array_lengthof(DefaultSpecialAcronyms) +
                             SpecialAcronyms.size());
     // No need to regex-escape the default acronyms.
     EscapedAcronyms.insert(EscapedAcronyms.end(),
                            std::begin(DefaultSpecialAcronyms),
                            std::end(DefaultSpecialAcronyms));
   } else {