[PATCH] D53547: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 22 19:05:17 PDT 2018


rsmith added a comment.

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.)



================
Comment at: clang/include/clang/Basic/Features.def:95
+FEATURE(objc_kindof, LangOpts.ObjC)
+FEATURE(objc_modules, LangOpts.ObjC &&LangOpts.Modules)
 FEATURE(objc_nonfragile_abi, LangOpts.ObjCRuntime.isNonFragile())
----------------
You touched this line last, so it's now your fault that there's missing space after the `&&` :)


================
Comment at: clang/lib/Basic/IdentifierTable.cpp:166-167
   // in non-arc mode.
-  if (LangOpts.ObjC2 && (Flags & KEYARC)) return KS_Enabled;
-  if (LangOpts.ObjC2 && (Flags & KEYOBJC2)) return KS_Enabled;
+  if (LangOpts.ObjC && (Flags & KEYARC)) return KS_Enabled;
+  if (LangOpts.ObjC && (Flags & KEYOBJC)) return KS_Enabled;
   if (LangOpts.ConceptsTS && (Flags & KEYCONCEPTS)) return KS_Enabled;
----------------
Would it make sense to fold `KEYOBJC` and `KEYARC` together?


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:1365
     Results.AddResult(Result("bool", CCP_Type +
-                             (LangOpts.ObjC1? CCD_bool_in_ObjC : 0)));
+                             (LangOpts.ObjC? CCD_bool_in_ObjC : 0)));
     Results.AddResult(Result("class", CCP_Type));
----------------
Space before `?` please.


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3335
   else if (MacroName.equals("bool"))
-    Priority = CCP_Type + (LangOpts.ObjC1? CCD_bool_in_ObjC : 0);
+    Priority = CCP_Type + (LangOpts.ObjC? CCD_bool_in_ObjC : 0);
 
----------------
Space before `?` please.


================
Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:408
     // non-Apple platforms, but for now it is needed.
-    m_compiler->getLangOpts().ObjC1 = true;
+    m_compiler->getLangOpts().ObjC = true;
     break;
----------------
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`?


Repository:
  rC Clang

https://reviews.llvm.org/D53547





More information about the cfe-commits mailing list