Skip to content

[clang][NFC] Move more functions from Sema to SemaObjC #97172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

Endilll
Copy link
Contributor

@Endilll Endilll commented Jun 29, 2024

This patch continues the effort to split Sema up, moving more function to SemaObjC. Additional context can be found in #84184 and #92682.

Highlights are getCurBlock() and PushBlockScope().

@Endilll Endilll added clang:frontend Language frontend issues, e.g. anything involving "Sema" objective-c labels Jun 29, 2024
@Endilll Endilll requested review from rjmccall and AaronBallman June 29, 2024 18:23
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:openmp OpenMP related changes to Clang labels Jun 29, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 29, 2024

@llvm/pr-subscribers-clang

Author: Vlad Serebrennikov (Endilll)

Changes

This patch continues the effort to split Sema up, moving more function to SemaObjC. Additional context can be found in #84184 and #92682.

Highlights are getCurBlock() and PushBlockScope().


Patch is 67.69 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/97172.diff

16 Files Affected:

  • (modified) clang/include/clang/Sema/Sema.h (-41)
  • (modified) clang/include/clang/Sema/SemaObjC.h (+45-4)
  • (modified) clang/lib/Parse/ParseExpr.cpp (+9-8)
  • (modified) clang/lib/Sema/Sema.cpp (-21)
  • (modified) clang/lib/Sema/SemaChecking.cpp (-118)
  • (modified) clang/lib/Sema/SemaCodeComplete.cpp (+5-4)
  • (modified) clang/lib/Sema/SemaDecl.cpp (+2-31)
  • (modified) clang/lib/Sema/SemaDeclObjC.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaExpr.cpp (+4-370)
  • (modified) clang/lib/Sema/SemaExprObjC.cpp (+380-3)
  • (modified) clang/lib/Sema/SemaLookup.cpp (+2-1)
  • (modified) clang/lib/Sema/SemaObjC.cpp (+174-10)
  • (modified) clang/lib/Sema/SemaOpenMP.cpp (+2-2)
  • (modified) clang/lib/Sema/SemaPseudoObject.cpp (+2-2)
  • (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+5-6)
  • (modified) clang/lib/Sema/TreeTransform.h (+6-6)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index ef4fc47567a7c..422149896fb36 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -694,7 +694,6 @@ class Sema final : public SemaBase {
   Scope *getScopeForContext(DeclContext *Ctx);
 
   void PushFunctionScope();
-  void PushBlockScope(Scope *BlockScope, BlockDecl *Block);
   sema::LambdaScopeInfo *PushLambdaScope();
 
   /// This is used to inform Sema what the current TemplateParameterDepth
@@ -736,9 +735,6 @@ class Sema final : public SemaBase {
 
   bool hasAnyUnrecoverableErrorsInThisFunction() const;
 
-  /// Retrieve the current block, if any.
-  sema::BlockScopeInfo *getCurBlock();
-
   /// Get the innermost lambda enclosing the current location, if any. This
   /// looks through intervening non-lambda scopes such as local functions and
   /// blocks.
@@ -2182,14 +2178,6 @@ class Sema final : public SemaBase {
 
   void CheckCastAlign(Expr *Op, QualType T, SourceRange TRange);
 
-  /// checkUnsafeAssigns - Check whether +1 expr is being assigned
-  /// to weak/__unsafe_unretained type.
-  bool checkUnsafeAssigns(SourceLocation Loc, QualType LHS, Expr *RHS);
-
-  /// checkUnsafeExprAssigns - Check whether +1 expr is being assigned
-  /// to weak/__unsafe_unretained expression.
-  void checkUnsafeExprAssigns(SourceLocation Loc, Expr *LHS, Expr *RHS);
-
   /// Emit \p DiagID if statement located on \p StmtLoc has a suspicious null
   /// statement as a \p Body, and it is located on the same line.
   ///
@@ -3853,8 +3841,6 @@ class Sema final : public SemaBase {
   void AddLaunchBoundsAttr(Decl *D, const AttributeCommonInfo &CI,
                            Expr *MaxThreads, Expr *MinBlocks, Expr *MaxBlocks);
 
-  enum class RetainOwnershipKind { NS, CF, OS };
-
   UuidAttr *mergeUuidAttr(Decl *D, const AttributeCommonInfo &CI,
                           StringRef UuidAsWritten, MSGuidDecl *GuidDecl);
 
@@ -5833,26 +5819,6 @@ class Sema final : public SemaBase {
 
   bool CheckCaseExpression(Expr *E);
 
-  //===------------------------- "Block" Extension ------------------------===//
-
-  /// ActOnBlockStart - This callback is invoked when a block literal is
-  /// started.
-  void ActOnBlockStart(SourceLocation CaretLoc, Scope *CurScope);
-
-  /// ActOnBlockArguments - This callback allows processing of block arguments.
-  /// If there are no arguments, this is still invoked.
-  void ActOnBlockArguments(SourceLocation CaretLoc, Declarator &ParamInfo,
-                           Scope *CurScope);
-
-  /// ActOnBlockError - If there is an error parsing a block, this callback
-  /// is invoked to pop the information about the block from the action impl.
-  void ActOnBlockError(SourceLocation CaretLoc, Scope *CurScope);
-
-  /// ActOnBlockStmtExpr - This is called when the body of a block statement
-  /// literal was successfully completed.  ^(int x){...}
-  ExprResult ActOnBlockStmtExpr(SourceLocation CaretLoc, Stmt *Body,
-                                Scope *CurScope);
-
   //===---------------------------- Clang Extensions ----------------------===//
 
   /// __builtin_convertvector(...)
@@ -6571,13 +6537,6 @@ class Sema final : public SemaBase {
   // Set of failed immediate invocations to avoid double diagnosing.
   llvm::SmallPtrSet<ConstantExpr *, 4> FailedImmediateInvocations;
 
-  /// List of SourceLocations where 'self' is implicitly retained inside a
-  /// block.
-  llvm::SmallVector<std::pair<SourceLocation, const BlockDecl *>, 1>
-      ImplicitlyRetainedSelfLocs;
-
-  void maybeExtendBlockObject(ExprResult &E);
-
 private:
   static BinaryOperatorKind ConvertTokenKindToBinaryOpcode(tok::TokenKind Kind);
 
diff --git a/clang/include/clang/Sema/SemaObjC.h b/clang/include/clang/Sema/SemaObjC.h
index 07c3c1a06be16..c962d48799ec5 100644
--- a/clang/include/clang/Sema/SemaObjC.h
+++ b/clang/include/clang/Sema/SemaObjC.h
@@ -179,6 +179,13 @@ class SemaObjC : public SemaBase {
   /// target type.
   void checkArrayLiteral(QualType TargetType, ObjCArrayLiteral *ArrayLiteral);
 
+  /// Retrieve the current block, if any.
+  sema::BlockScopeInfo *getCurBlock();
+
+  void PushBlockScope(Scope *BlockScope, BlockDecl *Block);
+
+  void diagnoseImplicitlyRetainedSelf();
+
 private:
   IdentifierInfo *Ident_NSError = nullptr;
 
@@ -571,6 +578,21 @@ class SemaObjC : public SemaBase {
 
   ObjCContainerDecl *getObjCDeclContext() const;
 
+  /// List of SourceLocations where 'self' is implicitly retained inside a
+  /// block.
+  llvm::SmallVector<std::pair<SourceLocation, const BlockDecl *>, 1>
+      ImplicitlyRetainedSelfLocs;
+
+  /// checkUnsafeAssigns - Check whether +1 expr is being assigned
+  /// to weak/__unsafe_unretained type.
+  bool checkUnsafeAssigns(SourceLocation Loc, QualType LHS, Expr *RHS);
+
+  /// checkUnsafeExprAssigns - Check whether +1 expr is being assigned
+  /// to weak/__unsafe_unretained expression.
+  void checkUnsafeExprAssigns(SourceLocation Loc, Expr *LHS, Expr *RHS);
+
+  enum class RetainOwnershipKind { NS, CF, OS };
+
 private:
   /// AddMethodToGlobalPool - Add an instance or factory method to the global
   /// pool. See descriptoin of AddInstanceMethodToGlobalPool.
@@ -925,6 +947,27 @@ class SemaObjC : public SemaBase {
   };
   ObjCLiteralKind CheckLiteralKind(Expr *FromE);
 
+  /// Do an explicit extend of the given block pointer if we're in ARC.
+  void maybeExtendBlockObject(ExprResult &E);
+
+  /// ActOnBlockStart - This callback is invoked when a block literal is
+  /// started.
+  void ActOnBlockStart(SourceLocation CaretLoc, Scope *CurScope);
+
+  /// ActOnBlockArguments - This callback allows processing of block arguments.
+  /// If there are no arguments, this is still invoked.
+  void ActOnBlockArguments(SourceLocation CaretLoc, Declarator &ParamInfo,
+                           Scope *CurScope);
+
+  /// ActOnBlockError - If there is an error parsing a block, this callback
+  /// is invoked to pop the information about the block from the action impl.
+  void ActOnBlockError(SourceLocation CaretLoc, Scope *CurScope);
+
+  /// ActOnBlockStmtExpr - This is called when the body of a block statement
+  /// literal was successfully completed.  ^(int x){...}
+  ExprResult ActOnBlockStmtExpr(SourceLocation CaretLoc, Stmt *Body,
+                                Scope *CurScope);
+
   ///@}
 
   //
@@ -1067,15 +1110,13 @@ class SemaObjC : public SemaBase {
   void handleExternallyRetainedAttr(Decl *D, const ParsedAttr &AL);
 
   void AddXConsumedAttr(Decl *D, const AttributeCommonInfo &CI,
-                        Sema::RetainOwnershipKind K,
-                        bool IsTemplateInstantiation);
+                        RetainOwnershipKind K, bool IsTemplateInstantiation);
 
   /// \return whether the parameter is a pointer to OSObject pointer.
   bool isValidOSObjectOutParameter(const Decl *D);
   bool checkNSReturnsRetainedReturnType(SourceLocation loc, QualType type);
 
-  Sema::RetainOwnershipKind
-  parsedAttrToRetainOwnershipKind(const ParsedAttr &AL);
+  RetainOwnershipKind parsedAttrToRetainOwnershipKind(const ParsedAttr &AL);
 
   ///@}
 };
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 9fc3cd73f73a0..ab744c68872d3 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -3733,7 +3733,7 @@ void Parser::ParseBlockId(SourceLocation CaretLoc) {
   MaybeParseGNUAttributes(DeclaratorInfo);
 
   // Inform sema that we are starting a block.
-  Actions.ActOnBlockArguments(CaretLoc, DeclaratorInfo, getCurScope());
+  Actions.ObjC().ActOnBlockArguments(CaretLoc, DeclaratorInfo, getCurScope());
 }
 
 /// ParseBlockLiteralExpression - Parse a block literal, which roughly looks
@@ -3761,7 +3761,7 @@ ExprResult Parser::ParseBlockLiteralExpression() {
                                   Scope::CompoundStmtScope | Scope::DeclScope);
 
   // Inform sema that we are starting a block.
-  Actions.ActOnBlockStart(CaretLoc, getCurScope());
+  Actions.ObjC().ActOnBlockStart(CaretLoc, getCurScope());
 
   // Parse the return type if present.
   DeclSpec DS(AttrFactory);
@@ -3786,14 +3786,14 @@ ExprResult Parser::ParseBlockLiteralExpression() {
       // If there was an error parsing the arguments, they may have
       // tried to use ^(x+y) which requires an argument list.  Just
       // skip the whole block literal.
-      Actions.ActOnBlockError(CaretLoc, getCurScope());
+      Actions.ObjC().ActOnBlockError(CaretLoc, getCurScope());
       return ExprError();
     }
 
     MaybeParseGNUAttributes(ParamInfo);
 
     // Inform sema that we are starting a block.
-    Actions.ActOnBlockArguments(CaretLoc, ParamInfo, getCurScope());
+    Actions.ObjC().ActOnBlockArguments(CaretLoc, ParamInfo, getCurScope());
   } else if (!Tok.is(tok::l_brace)) {
     ParseBlockId(CaretLoc);
   } else {
@@ -3823,7 +3823,7 @@ ExprResult Parser::ParseBlockLiteralExpression() {
     MaybeParseGNUAttributes(ParamInfo);
 
     // Inform sema that we are starting a block.
-    Actions.ActOnBlockArguments(CaretLoc, ParamInfo, getCurScope());
+    Actions.ObjC().ActOnBlockArguments(CaretLoc, ParamInfo, getCurScope());
   }
 
 
@@ -3831,16 +3831,17 @@ ExprResult Parser::ParseBlockLiteralExpression() {
   if (!Tok.is(tok::l_brace)) {
     // Saw something like: ^expr
     Diag(Tok, diag::err_expected_expression);
-    Actions.ActOnBlockError(CaretLoc, getCurScope());
+    Actions.ObjC().ActOnBlockError(CaretLoc, getCurScope());
     return ExprError();
   }
 
   StmtResult Stmt(ParseCompoundStatementBody());
   BlockScope.Exit();
   if (!Stmt.isInvalid())
-    Result = Actions.ActOnBlockStmtExpr(CaretLoc, Stmt.get(), getCurScope());
+    Result =
+        Actions.ObjC().ActOnBlockStmtExpr(CaretLoc, Stmt.get(), getCurScope());
   else
-    Actions.ActOnBlockError(CaretLoc, getCurScope());
+    Actions.ObjC().ActOnBlockError(CaretLoc, getCurScope());
   return Result;
 }
 
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 3f8f2f027172d..e32192af436cf 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -2204,12 +2204,6 @@ void Sema::PushFunctionScope() {
     OpenMP().pushOpenMPFunctionRegion();
 }
 
-void Sema::PushBlockScope(Scope *BlockScope, BlockDecl *Block) {
-  FunctionScopes.push_back(new BlockScopeInfo(getDiagnostics(),
-                                              BlockScope, Block));
-  CapturingFunctionScopes++;
-}
-
 LambdaScopeInfo *Sema::PushLambdaScope() {
   LambdaScopeInfo *const LSI = new LambdaScopeInfo(getDiagnostics());
   FunctionScopes.push_back(LSI);
@@ -2382,21 +2376,6 @@ void Sema::setFunctionHasMustTail() {
     FunctionScopes.back()->setHasMustTail();
 }
 
-BlockScopeInfo *Sema::getCurBlock() {
-  if (FunctionScopes.empty())
-    return nullptr;
-
-  auto CurBSI = dyn_cast<BlockScopeInfo>(FunctionScopes.back());
-  if (CurBSI && CurBSI->TheDecl &&
-      !CurBSI->TheDecl->Encloses(CurContext)) {
-    // We have switched contexts due to template instantiation.
-    assert(!CodeSynthesisContexts.empty());
-    return nullptr;
-  }
-
-  return CurBSI;
-}
-
 FunctionScopeInfo *Sema::getEnclosingFunction() const {
   if (FunctionScopes.empty())
     return nullptr;
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 005fbfd42a8ab..784606368b3ae 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13457,124 +13457,6 @@ void Sema::CheckArrayAccess(const Expr *expr) {
   }
 }
 
-static bool checkUnsafeAssignLiteral(Sema &S, SourceLocation Loc,
-                                     Expr *RHS, bool isProperty) {
-  // Check if RHS is an Objective-C object literal, which also can get
-  // immediately zapped in a weak reference.  Note that we explicitly
-  // allow ObjCStringLiterals, since those are designed to never really die.
-  RHS = RHS->IgnoreParenImpCasts();
-
-  // This enum needs to match with the 'select' in
-  // warn_objc_arc_literal_assign (off-by-1).
-  SemaObjC::ObjCLiteralKind Kind = S.ObjC().CheckLiteralKind(RHS);
-  if (Kind == SemaObjC::LK_String || Kind == SemaObjC::LK_None)
-    return false;
-
-  S.Diag(Loc, diag::warn_arc_literal_assign)
-    << (unsigned) Kind
-    << (isProperty ? 0 : 1)
-    << RHS->getSourceRange();
-
-  return true;
-}
-
-static bool checkUnsafeAssignObject(Sema &S, SourceLocation Loc,
-                                    Qualifiers::ObjCLifetime LT,
-                                    Expr *RHS, bool isProperty) {
-  // Strip off any implicit cast added to get to the one ARC-specific.
-  while (ImplicitCastExpr *cast = dyn_cast<ImplicitCastExpr>(RHS)) {
-    if (cast->getCastKind() == CK_ARCConsumeObject) {
-      S.Diag(Loc, diag::warn_arc_retained_assign)
-        << (LT == Qualifiers::OCL_ExplicitNone)
-        << (isProperty ? 0 : 1)
-        << RHS->getSourceRange();
-      return true;
-    }
-    RHS = cast->getSubExpr();
-  }
-
-  if (LT == Qualifiers::OCL_Weak &&
-      checkUnsafeAssignLiteral(S, Loc, RHS, isProperty))
-    return true;
-
-  return false;
-}
-
-bool Sema::checkUnsafeAssigns(SourceLocation Loc,
-                              QualType LHS, Expr *RHS) {
-  Qualifiers::ObjCLifetime LT = LHS.getObjCLifetime();
-
-  if (LT != Qualifiers::OCL_Weak && LT != Qualifiers::OCL_ExplicitNone)
-    return false;
-
-  if (checkUnsafeAssignObject(*this, Loc, LT, RHS, false))
-    return true;
-
-  return false;
-}
-
-void Sema::checkUnsafeExprAssigns(SourceLocation Loc,
-                              Expr *LHS, Expr *RHS) {
-  QualType LHSType;
-  // PropertyRef on LHS type need be directly obtained from
-  // its declaration as it has a PseudoType.
-  ObjCPropertyRefExpr *PRE
-    = dyn_cast<ObjCPropertyRefExpr>(LHS->IgnoreParens());
-  if (PRE && !PRE->isImplicitProperty()) {
-    const ObjCPropertyDecl *PD = PRE->getExplicitProperty();
-    if (PD)
-      LHSType = PD->getType();
-  }
-
-  if (LHSType.isNull())
-    LHSType = LHS->getType();
-
-  Qualifiers::ObjCLifetime LT = LHSType.getObjCLifetime();
-
-  if (LT == Qualifiers::OCL_Weak) {
-    if (!Diags.isIgnored(diag::warn_arc_repeated_use_of_weak, Loc))
-      getCurFunction()->markSafeWeakUse(LHS);
-  }
-
-  if (checkUnsafeAssigns(Loc, LHSType, RHS))
-    return;
-
-  // FIXME. Check for other life times.
-  if (LT != Qualifiers::OCL_None)
-    return;
-
-  if (PRE) {
-    if (PRE->isImplicitProperty())
-      return;
-    const ObjCPropertyDecl *PD = PRE->getExplicitProperty();
-    if (!PD)
-      return;
-
-    unsigned Attributes = PD->getPropertyAttributes();
-    if (Attributes & ObjCPropertyAttribute::kind_assign) {
-      // when 'assign' attribute was not explicitly specified
-      // by user, ignore it and rely on property type itself
-      // for lifetime info.
-      unsigned AsWrittenAttr = PD->getPropertyAttributesAsWritten();
-      if (!(AsWrittenAttr & ObjCPropertyAttribute::kind_assign) &&
-          LHSType->isObjCRetainableType())
-        return;
-
-      while (ImplicitCastExpr *cast = dyn_cast<ImplicitCastExpr>(RHS)) {
-        if (cast->getCastKind() == CK_ARCConsumeObject) {
-          Diag(Loc, diag::warn_arc_retained_property_assign)
-          << RHS->getSourceRange();
-          return;
-        }
-        RHS = cast->getSubExpr();
-      }
-    } else if (Attributes & ObjCPropertyAttribute::kind_weak) {
-      if (checkUnsafeAssignObject(*this, Loc, Qualifiers::OCL_Weak, RHS, true))
-        return;
-    }
-  }
-}
-
 //===--- CHECK: Empty statement body (-Wempty-body) ---------------------===//
 
 static bool ShouldDiagnoseEmptyStmtBody(const SourceManager &SourceMgr,
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index cd1c5f9391ccd..0801cedf675fe 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -413,7 +413,7 @@ void PreferredTypeBuilder::enterReturn(Sema &S, SourceLocation Tok) {
   if (!Enabled)
     return;
   if (isa<BlockDecl>(S.CurContext)) {
-    if (sema::BlockScopeInfo *BSI = S.getCurBlock()) {
+    if (sema::BlockScopeInfo *BSI = S.ObjC().getCurBlock()) {
       ComputeType = nullptr;
       Type = BSI->ReturnType;
       ExpectedLoc = Tok;
@@ -2473,9 +2473,10 @@ AddOrdinaryNameResults(SemaCodeCompletion::ParserCompletionContext CCC,
       ReturnType = Function->getReturnType();
     else if (const auto *Method = dyn_cast<ObjCMethodDecl>(SemaRef.CurContext))
       ReturnType = Method->getReturnType();
-    else if (SemaRef.getCurBlock() &&
-             !SemaRef.getCurBlock()->ReturnType.isNull())
-      ReturnType = SemaRef.getCurBlock()->ReturnType;;
+    else if (SemaRef.ObjC().getCurBlock() &&
+             !SemaRef.ObjC().getCurBlock()->ReturnType.isNull())
+      ReturnType = SemaRef.ObjC().getCurBlock()->ReturnType;
+    ;
     if (ReturnType.isNull() || ReturnType->isVoidType()) {
       Builder.AddTypedTextChunk("return");
       Builder.AddChunk(CodeCompletionString::CK_SemiColon);
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 029ccf944c513..deb161ee352be 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13782,7 +13782,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
     VDecl->setType(DclT);
 
   if (!VDecl->isInvalidDecl()) {
-    checkUnsafeAssigns(VDecl->getLocation(), VDecl->getType(), Init);
+    ObjC().checkUnsafeAssigns(VDecl->getLocation(), VDecl->getType(), Init);
 
     if (VDecl->hasAttr<BlocksAttr>())
       ObjC().checkRetainCycles(VDecl, Init);
@@ -16016,35 +16016,6 @@ class ExitFunctionBodyRAII {
   bool IsLambda = false;
 };
 
-static void diagnoseImplicitlyRetainedSelf(Sema &S) {
-  llvm::DenseMap<const BlockDecl *, bool> EscapeInfo;
-
-  auto IsOrNestedInEscapingBlock = [&](const BlockDecl *BD) {
-    if (EscapeInfo.count(BD))
-      return EscapeInfo[BD];
-
-    bool R = false;
-    const BlockDecl *CurBD = BD;
-
-    do {
-      R = !CurBD->doesNotEscape();
-      if (R)
-        break;
-      CurBD = CurBD->getParent()->getInnermostBlockDecl();
-    } while (CurBD);
-
-    return EscapeInfo[BD] = R;
-  };
-
-  // If the location where 'self' is implicitly retained is inside a escaping
-  // block, emit a diagnostic.
-  for (const std::pair<SourceLocation, const BlockDecl *> &P :
-       S.ImplicitlyRetainedSelfLocs)
-    if (IsOrNestedInEscapingBlock(P.second))
-      S.Diag(P.first, diag::warn_implicitly_retains_self)
-          << FixItHint::CreateInsertion(P.first, "self->");
-}
-
 static bool methodHasName(const FunctionDecl *FD, StringRef Name) {
   return isa<CXXMethodDecl>(FD) && FD->param_empty() &&
          FD->getDeclName().isIdentifier() && FD->getName() == Name;
@@ -16392,7 +16363,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
         FSI->ObjCWarnForNoInitDelegation = false;
       }
 
-      diagnoseImplicitlyRetainedSelf(*this);
+      ObjC().diagnoseImplicitlyRetainedSelf();
     } else {
       // Parsing the function declaration failed in some way. Pop the fake scope
       // we pushed on.
diff --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index 807453400abdd..d60d9968b7957 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -368,7 +368,7 @@ HasExplicitOwnershipAttr(Sema &S, ParmVarDecl *Param) {
 /// and user declared, in the method definition's AST.
 void SemaObjC::ActOnStartOfObjCMethodDef(Scope *FnBodyScope, Decl *D) {
   ASTContext &Context = getASTContext();
-  SemaRef.ImplicitlyRetainedSelfLocs.clear();
+  ImplicitlyRetainedSelfLocs.clear();
   assert((SemaRef.getCurMethodDecl() == nullptr) && "Methodparsing confused");
   ObjCMethodDecl *MDecl = dyn_cast_or_null<ObjCMethodDecl>(D);
 
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index db44cfe1288b6..9bc771a34073a 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -1070,7 +1070,7 @@ ExprResult Sema::DefaultVariadicArgumentPromotion(Expr *E, VariadicCallType CT,
 
   // Copy blocks to the heap.
   if (ExprRes.get()->getType()->isBlockPointerType())
-    maybeExtendBlockObject(ExprRes);
+    ObjC().maybeExtendBlockObject(ExprRes);
 
   E = ExprRes.get();
 
@@ -7306,20 +7306,6 @@ Sema::BuildInitList(SourceLocation LBraceLoc, MultiExprArg InitArgList,
   return E;
 }
 
-/// Do an explicit extend of the given block pointer if we're in ARC.
-void Sema::maybeExtendBlockObject(ExprResult &E) {
-  assert(E.get()->getType()->is...
[truncated]

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I think these changes are defensible, I think I would prefer to not move all of the blocks logic into SemaObjC, but I'd love to hear if @rjmccall agrees with my rationale and conclusion.

Blocks are largely an Objective-C feature, so putting them together makes sense from that perspective. But Blocks are a C and C++ language extension that Objective-C and Objective-C++ pick up automatically; we even document it as such in https://clang.llvm.org/docs/BlockLanguageSpec.html I think that placing blocks into Objective-C semantics makes that point less clear, but it is an important distinction. For example, WG21 is now looking at the syntax used for reflection because it was pointed out that the proposed syntax conflicts with a conforming extension in C++ in a way that would be hard for us to support.

WDYT?

@rjmccall
Copy link
Contributor

I agree that blocks don't belong in the ObjC segment.

@Endilll Endilll closed this Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category objective-c
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants