[cfe-commits] r145774 - in /cfe/trunk: include/clang/AST/Decl.h include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp lib/Sema/SemaStmt.cpp lib/Sema/TreeTransform.h test/Sema/block-explicit-noreturn-type.c test/Sema/block-return.c test/SemaCXX/instantiate-blocks.cpp

[Douglas Gregor]

On Dec 3, 2011, at 9:47 AM, Fariborz Jahanian wrote:

> Author: fjahanian
> Date: Sat Dec  3 11:47:53 2011
> New Revision: 145774
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=145774&view=rev
> Log:
> If block literal return type is not specified, return type of the block is 
> inferred from return types. All the return statements have to agree about the type.
> // rdar://10466373
> 
> Added:
>    cfe/trunk/test/Sema/block-explicit-noreturn-type.c
> Modified:
>    cfe/trunk/include/clang/AST/Decl.h
>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>    cfe/trunk/lib/Sema/SemaExpr.cpp
>    cfe/trunk/lib/Sema/SemaStmt.cpp
>    cfe/trunk/lib/Sema/TreeTransform.h
>    cfe/trunk/test/Sema/block-return.c
>    cfe/trunk/test/SemaCXX/instantiate-blocks.cpp
> 
> Modified: cfe/trunk/include/clang/AST/Decl.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=145774&r1=145773&r2=145774&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/Decl.h (original)
> +++ cfe/trunk/include/clang/AST/Decl.h Sat Dec  3 11:47:53 2011
> @@ -2981,6 +2981,7 @@
>   // FIXME: This can be packed into the bitfields in Decl.
>   bool IsVariadic : 1;
>   bool CapturesCXXThis : 1;
> +  bool BlockMissingReturnType : 1;
>   /// ParamInfo - new[]'d array of pointers to ParmVarDecls for the formal
>   /// parameters of this function.  This is null if a prototype or if there are
>   /// no formals.
> @@ -2997,6 +2998,7 @@
>   BlockDecl(DeclContext *DC, SourceLocation CaretLoc)
>     : Decl(Block, DC, CaretLoc), DeclContext(Block),
>       IsVariadic(false), CapturesCXXThis(false),
> +      BlockMissingReturnType(true),
>       ParamInfo(0), NumParams(0), Body(0),
>       SignatureAsWritten(0), Captures(0), NumCaptures(0) {}
> 
> @@ -3054,6 +3056,8 @@
>   capture_const_iterator capture_end() const { return Captures + NumCaptures; }
> 
>   bool capturesCXXThis() const { return CapturesCXXThis; }
> +  bool blockMissingReturnType() const { return BlockMissingReturnType; }
> +  void setBlockMissingReturnType(bool val) { BlockMissingReturnType = val; }
> 
>   bool capturesVariable(const VarDecl *var) const;
> 
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=145774&r1=145773&r2=145774&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat Dec  3 11:47:53 2011
> @@ -4061,6 +4061,9 @@
>   "volatile and restrict|const, volatile, and restrict}5 vs "
>   "%select{none|const|restrict|const and restrict|volatile|const and volatile|"
>   "volatile and restrict|const, volatile, and restrict}6)}4">;
> +def err_typecheck_missing_return_type_incompatible : Error<
> +  "return type %0 must match previous return type %1 when block"
> +  " literal has unspecified explicit return type">;
> 
> def warn_incompatible_qualified_id : Warning<
>   "%select{assigning to|passing|returning|converting|initializing|sending|casting}2"
> 
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=145774&r1=145773&r2=145774&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Sat Dec  3 11:47:53 2011
> @@ -8715,8 +8715,10 @@
>   // return type.  TODO:  what should we do with declarators like:
>   //   ^ * { ... }
>   // If the answer is "apply template argument deduction"....
> -  if (RetTy != Context.DependentTy)
> +  if (RetTy != Context.DependentTy) {
>     CurBlock->ReturnType = RetTy;
> +    CurBlock->TheDecl->setBlockMissingReturnType(false);
> +  }
> 
>   // Push block parameters from the declarator if we had them.
>   SmallVector<ParmVarDecl*, 8> Params;
> 
> Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=145774&r1=145773&r2=145774&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaStmt.cpp Sat Dec  3 11:47:53 2011
> @@ -1809,6 +1809,16 @@
>   } else if (!RetValExp) {
>     return StmtError(Diag(ReturnLoc, diag::err_block_return_missing_expr));
>   } else if (!RetValExp->isTypeDependent()) {
> +    if (CurBlock->TheDecl->blockMissingReturnType()) {
> +      // when block's return type is not specified, all return types
> +      // must strictly match.
> +      if (Context.getCanonicalType(FnRetType) != 
> +          Context.getCanonicalType(RetValExp->getType())) {

Context.hasSameType(FnRetType, RetValExp->getType()) would be a little cleaner. 

More importantly, I think this is the wrong place for this check, because the code isn't performing function or array lvalue conversions before checking the type of the return expression. I suggest making sure that blocks with inferred return types always go into this code:

  if (CurBlock->ReturnType.isNull()) {
    if (RetValExp) {
      // Don't call UsualUnaryConversions(), since we don't want to do
      // integer promotions here.
      ExprResult Result = DefaultFunctionArrayLvalueConversion(RetValExp);
      if (Result.isInvalid())
        return StmtError();
      RetValExp = Result.take();

      if (!RetValExp->isTypeDependent()) {
        CurBlock->ReturnType = RetValExp->getType();
        if (BlockDeclRefExpr *CDRE = dyn_cast<BlockDeclRefExpr>(RetValExp)) {
          // We have to remove a 'const' added to copied-in variable which was
          // part of the implementation spec. and not the actual qualifier for
          // the variable.
          if (CDRE->isConstQualAdded())
            CurBlock->ReturnType.removeLocalConst(); // FIXME: local???
        }
      } else
        CurBlock->ReturnType = Context.DependentTy;
    } else
      CurBlock->ReturnType = Context.VoidTy;
  }

so they all get the same adjustments before checking against previously-deduced return types.

	- Doug

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111203/e28a7ce6/attachment.html>