[Sema][ObjC] Add support for attribute "noescape"

[ahatanak]

Summary

This patch adds support for attribute "noescape", which is used to tell the compiler that a block passed to a function will not be called after the function returns.

To ensure that the block does not escape, clang imposes the following restrictions on its usage:

• Cannot be passed to a function expecting an escaping block
• Cannot be returned from a function
• Cannot be captured by a block
• Cannot be assigned to a variable

There are other improvements we can make to Sema and CodeGen if the compiler can tell whether a block escapes or not, which should follow this patch.

rdar://problem/19886775

[aaron.ballman]

include/clang/Basic/Attr.td

1222 ↗

(On Diff #95679)

I do not think GCC supports this attribute, so that should be using a GNU spelling rather than a GCC spelling.

1223 ↗

(On Diff #95679)

No need to specify the last two arguments, they should be handled by default already.

include/clang/Basic/AttrDocs.td

118 ↗

(On Diff #95679)

Please wrap to 80 columns.

"block parameter" is a bit strange -- I assumed that meant parameter to a block, not a function parameter of block type. May want to clarify.

Should also clarify "the block will not be invoked after the function returns." Does this code produce undefined behavior?

typedef void (^BlockTy)();
void nonescapingFunc(__attribute__((noescape)) BlockTy);
void escapingFunc(BlockTy);

void callerFunc(__attribute__((noescape)) BlockTy block) {
  nonescapingFunc(block); // OK
  escapingFunc(block);    // error: parameter is not annotated with noescape
}

void f(void) {
  BlockTy Blk = ^{};
  callerFunc(Blk);
  Blk();
}

include/clang/Basic/DiagnosticSemaKinds.td

3094 ↗

(On Diff #95679)

expecting an escaping argument

Also, someday we should make non-foo vs nonfoo a consistent decision in our diagnostics. We seem to use both forms.

3106 ↗

(On Diff #95679)

Should quote noescape.

lib/Sema/SemaChecking.cpp

2545 ↗

(On Diff #95679)

Space before the asterisk.

2548 ↗

(On Diff #95679)

Please don't use auto here.

2559 ↗

(On Diff #95679)

Don't use auto here.

2562 ↗

(On Diff #95679)

No need to use getName() here, you can just pass in the declaration directly. That means you should also remove the quotes from the diagnostic so it does not get double-quoted.

2567 ↗

(On Diff #95679)

What about functions that have no prototype (they're not variadic, but they accept any number of arguments)? Should include a test for this.

lib/Sema/SemaDeclAttr.cpp

1520–1522 ↗

(On Diff #95679)

Is there ever a case where this if statement will early return? I think you can just use cast<ParmVarDecl>(D)->getType() below.

lib/Sema/SemaExpr.cpp

11180 ↗

(On Diff #95679)

Why dyn_cast_or_null? I didn't think IgnoreImpCasts() could return null.

11181 ↗

(On Diff #95679)

Don't use auto.

11184 ↗

(On Diff #95679)

Can drop the getName() call.

13682 ↗

(On Diff #95679)

Can drop the getName() call.

lib/Sema/SemaStmt.cpp

3203 ↗

(On Diff #95679)

Don't use auto.

3205 ↗

(On Diff #95679)

Can drop getName().

[jroelofs]

This doesn't forbid assigning them to block properties... is that intentional?

typedef void (^Block)(int);

@interface Foo
@property Block B;
@end

void foo(Foo *f, Block __attribute__((noescape)) b) {
  f.B = b;
}

include/clang/Basic/AttrDocs.td

123 ↗

(On Diff #95679)

Is it helpful to forbid taking the address of them, too?

[TheRealAdamKemp]

include/clang/Basic/AttrDocs.td

122 ↗

(On Diff #95679)

This may be too restrictive in some cases. Consider this:

typedef void (^BlockTy)();
void nonescapingFunc(__attribute__((noescape)) BlockTy);

void callerFunc(__attribute__((noescape)) BlockTy block) {
  nonescapingFunc(^{ block(); });
}

It sounds like the above code would give an error, but the capturing block is also nonescaping, which means it should be allowed. This is a useful pattern, and disallowing it would make these attributes very cumbersome.

The code could also be written like this:

typedef void (^BlockTy)();
void nonescapingFunc(__attribute__((noescape)) BlockTy);

void callerFunc(__attribute__((noescape)) BlockTy block) {
  BlockTy wrapBlock = ^{
    block();
  };
  nonescapingFunc(wrapBlock);
}

Again, I would expect that to not give an error or at least be able to decorate the declaration of wrapBlock to indicate that it is also nonescaping (can this attribute be applied to locals or only function arguments?).

[rjmccall]

The rule we actually want is that no reference to the block derived from the parameter value will survive after the function returns. You can include examples of things that would cause potential escapes, but trying to actually lock down the list seems ill-advised.

Are you sure you want to try to enforce this in the frontend? In Swift, we discovered that we needed some way of escaping the static checking in order to make this practical, because there are a lot of patterns that locally look like escapes but really aren't (e.g. passing the block to dispatch_async and then waiting for that to complete). Seems more like a static analysis than a frontend warning.

[ahatanak]

In D32210#730553, @jroelofs wrote:

This doesn't forbid assigning them to block properties... is that intentional?

typedef void (^Block)(int);

@interface Foo
@property Block B;
@end

void foo(Foo *f, Block __attribute__((noescape)) b) {
  f.B = b;
}

No, it's not intentional. The code to check assignments of noescape blocks probably had to go to ActOnBinOp or BuildBinOp to forbid this too.

include/clang/Basic/AttrDocs.td

122 ↗

(On Diff #95679)

I didn't think forbidding capturing noescape blocks would be too restrictive in practice, but I see your point. If it's a common pattern, we probably shouldn't forbid that.

lib/Sema/SemaChecking.cpp

2567 ↗

(On Diff #95679)

You are right, this assertion should check for functions without prototypes too.

[ahatanak]

In D32210#730777, @rjmccall wrote:

The rule we actually want is that no reference to the block derived from the parameter value will survive after the function returns. You can include examples of things that would cause potential escapes, but trying to actually lock down the list seems ill-advised.

Are you sure you want to try to enforce this in the frontend? In Swift, we discovered that we needed some way of escaping the static checking in order to make this practical, because there are a lot of patterns that locally look like escapes but really aren't (e.g. passing the block to dispatch_async and then waiting for that to complete). Seems more like a static analysis than a frontend warning.

I'm actually not so sure we want to enforce this in the front-end. I was following what I thought Swift was doing, which is to reject code that can potentially cause noescape closures to escape. But that might be too restrictive and can reject legitimate code in some cases as Adam pointed out. I agree that static analysis would probably be better at detecting usages of noescape blocks that are likely to cause them to escape.

Do you think we should remove all the restrictions I listed and instead count on the users to ensure noescape blocks do not escape?

[rjmccall]

In D32210#731192, @ahatanak wrote:

In D32210#730777, @rjmccall wrote:

The rule we actually want is that no reference to the block derived from the parameter value will survive after the function returns. You can include examples of things that would cause potential escapes, but trying to actually lock down the list seems ill-advised.

Are you sure you want to try to enforce this in the frontend? In Swift, we discovered that we needed some way of escaping the static checking in order to make this practical, because there are a lot of patterns that locally look like escapes but really aren't (e.g. passing the block to dispatch_async and then waiting for that to complete). Seems more like a static analysis than a frontend warning.

I'm actually not so sure we want to enforce this in the front-end. I was following what I thought Swift was doing, which is to reject code that can potentially cause noescape closures to escape. But that might be too restrictive and can reject legitimate code in some cases as Adam pointed out. I agree that static analysis would probably be better at detecting usages of noescape blocks that are likely to cause them to escape.

Do you think we should remove all the restrictions I listed and instead count on the users to ensure noescape blocks do not escape?

Yes, I think so. You'll need to document that, of course.

John.

[ahatanak]

Address review comments.

include/clang/Basic/AttrDocs.td

118 ↗

(On Diff #95679)

The code above doesn't produce undefined behavior as long as escapingFunc doesn't cause the block to escape. It is OK to declare a variable to hold the block and invoke it later after callerFunc returns. If a block literal is directly passed to callerFunc, the block will not be invoked after the function returns.

void f(void) {
  BlockTy Blk = ^{};
  callerFunc(Blk); // Blk can be invoked after callerFunc returns.
  Blk();
  callerFunc(^{}); // the block passed cannot be invoked after callerFunc returns.
}

[aaron.ballman]

The attribute is not used anywhere; the initial utility should be part of the patch introducing the attribute, unless that utility makes the patch very large.

lib/Sema/SemaDeclAttr.cpp

1522–1523 ↗

(On Diff #96637)

I don't think the type checking here is correct, at least according to the documentation. For instance, you don't appear to care whether the parameter is an array of block pointers vs an array of ints. Similar for the other composite types.

test/SemaObjCXX/noescape.mm

8 ↗

(On Diff #96637)

You should also have a test ensuring the attribute diagnoses when given an argument.

[rjmccall]

I agree with Aaron, it would be nice if this had some immediate effect. One obvious suggestion: take advantage of it in code generation. LLVM already has a parameter attribute called "nocapture" which conveys exactly the right semantics for noescape pointers and references. For block pointers, nocapture is a weaker statement than noescape, because noescape also restricts block copies, but it's still correct.

include/clang/Basic/AttrDocs.td

120 ↗

(On Diff #96637)

The implementation in Sema allows normal pointers and references in addition to block pointers. Assuming that's intended, it should be documented.

You should also document that *copies* of the block are also not allowed to escape. That's special behavior for block pointers.

144 ↗

(On Diff #96637)

Typo.

lib/Sema/SemaDeclAttr.cpp

1522–1523 ↗

(On Diff #96637)

A member pointer is not really like the others. Member pointers are always global constants, like function pointers; there's no meaningful scope for them to escape from.

Also, parameters never have array type. The type of a ParmVarDecl will be the decayed type, i.e. a pointer.

Also, you should check for an invalid decl and avoid checking the type, because that's likely a cascading failure. Test case: make a parameter with a bad type specifier.

[ahatanak]

Address review comments.

• Allow attaching "noescape" to pointers other than block pointers. Update the docs accordingly.
• Attach attribute "nocapture" to parameters that are annotated with "noescape".
• Call Sema::isValidPointerAttrType to determine whether "noescape" can be applied to a parameter. Also, use the existing warning "warn_attribute_pointers_only" rather than defining a new warning that will be used just for noescape.

I also thought about what else we can do in the front-end when a parameter has 'noescape". One idea is to do something similar to what r301667 did and omit emitting retains and releases of block captures when the block is passed to a function taking a noescape parameter. If that is viable, I can look into it after committing this patch.

[rjmccall]

In D32210#810292, @ahatanak wrote:

Address review comments.

• Allow attaching "noescape" to pointers other than block pointers. Update the docs accordingly.
• Attach attribute "nocapture" to parameters that are annotated with "noescape".
• Call Sema::isValidPointerAttrType to determine whether "noescape" can be applied to a parameter. Also, use the existing warning "warn_attribute_pointers_only" rather than defining a new warning that will be used just for noescape.

I also thought about what else we can do in the front-end when a parameter has 'noescape". One idea is to do something similar to what r301667 did and omit emitting retains and releases of block captures when the block is passed to a function taking a noescape parameter. If that is viable, I can look into it after committing this patch.

Hmm. Unfortunately, I'm not sure that's valid. The retains and releases of block captures don't protect against anything related to escaping the block; they protect against the original variables being modified during the lifetime of the block. It is true that a block literal passed to a noescape parameter has a shorter effective lifetime — we know for certain that it will be unused after the call, and ARC only promises that a value in an imprecise-lifetime strong variable like a block capture will be valid until the last load of that value from the variable. But that duration is still long enough for someone to modify the original variable in a way that is properly sequenced after the formation of the block.

Now, if you could prove that the variable was not modified for the duration of the call, that would be sufficient. And that would be easy to do in the common case by just proving that the address of the variable never escapes. Unfortunately, we don't have that information readily available because Sema doesn't collect it.

There are some other ways you could optimize blocks that are known not to escape, though. One big caveat is that you have to make sure the block behaves reasonably in response to being copied, because being noescape doesn't guarantee that the callee won't try to copy the block. However:

• Copying a global block is always a no-op. If you gave the non-escaping stack block a global block isa, that would allow the blocks runtime to avoid doing extra work when a non-escaping block is spuriously copied, and it would allow the compiler to completely avoid emitting copy and destroy helpers for the block. Please clear this with Greg Parker first, though.
• Even if that's not possible, you can still avoid emitting copy and destroy helpers in the common case where none of the captures are address-sensitive, i.e. __weak references or non-trivally-copyable C++ types, because the memcpy from the original block will be sufficient for correctness.
• You can just capture a reference to an outer block instead of copying anything that it captures.
• block variables which are only captured by non-escaping blocks are themselves known not to escape. (But remember that a non-escaping block can create an escaping block that captures the block variable!) Because block variable copies are only ever kicked off by block copy helpers, and you won't be generating those (or at least won't be asking them to copy your block variables for you), you also know that such variables can't be copied. That in turn means you can completely drop all the nonsense that only exists to supporting the lazy copying of block variables, like the block header and copy/destroy helpers and forwarding. Just generate the variable like a normal local variable and capture it by reference.

John.

include/clang/AST/Type.h

3156 ↗

(On Diff #106735)

Oh! I hadn't noticed that you were adding this to ExtParameterInfo. You're modifying the function type system; there's a lot of complexity to do that properly which you haven't done in this patch at all. That's especially true because, unlike all these other ExtParameterInfo cases, there's a natural subtyping rule for escaping parameters: a function type that takes a non-escaping parameter should be implicitly convertible to be a function type that takes an escaping parameter. You will also need to update a bunch of things, including the type printer, the mangler, the C++ function conversion rules, the C type compatibility rules, the redeclaration type matcher, and maybe even template argument deduction. It's a big thing to do.

You will want Sema tests in C, ObjC, C++, and ObjC++.

lib/CodeGen/CGCall.cpp

2096 ↗

(On Diff #106735)

You should make sure that building a CGFunctionInfo with an ObjCMethodDecl* collects this from the parameters. (And add tests!)

For blocks, at least, the ObjC method case is arguably more important than the C function case.

[ahatanak]

In D32210#810508, @rjmccall wrote:

In D32210#810292, @ahatanak wrote:

Address review comments.

• Allow attaching "noescape" to pointers other than block pointers. Update the docs accordingly.
• Attach attribute "nocapture" to parameters that are annotated with "noescape".
• Call Sema::isValidPointerAttrType to determine whether "noescape" can be applied to a parameter. Also, use the existing warning "warn_attribute_pointers_only" rather than defining a new warning that will be used just for noescape.

I also thought about what else we can do in the front-end when a parameter has 'noescape". One idea is to do something similar to what r301667 did and omit emitting retains and releases of block captures when the block is passed to a function taking a noescape parameter. If that is viable, I can look into it after committing this patch.

Hmm. Unfortunately, I'm not sure that's valid. The retains and releases of block captures don't protect against anything related to escaping the block; they protect against the original variables being modified during the lifetime of the block. It is true that a block literal passed to a noescape parameter has a shorter effective lifetime — we know for certain that it will be unused after the call, and ARC only promises that a value in an imprecise-lifetime strong variable like a block capture will be valid until the last load of that value from the variable. But that duration is still long enough for someone to modify the original variable in a way that is properly sequenced after the formation of the block.

Now, if you could prove that the variable was not modified for the duration of the call, that would be sufficient. And that would be easy to do in the common case by just proving that the address of the variable never escapes. Unfortunately, we don't have that information readily available because Sema doesn't collect it.

OK, so I guess the optimization isn't valid in the following case, for example:

void foo3(id);

id __strong *gp;

void foo2(id a) {
  gp = &a;
  nonescapingFunc(^{ foo3(a); }); // this function can modify "a" before the block is executed.
}

There are some other ways you could optimize blocks that are known not to escape, though. One big caveat is that you have to make sure the block behaves reasonably in response to being copied, becase being noescape doesn't guarantee that the callee won't try to copy the block. However:

I didn't understand the following optimizations. I thought clang doesn't emit copy and destroy helper for global blocks with or without noescape?

• Copying a global block is always a no-op. If you gave the non-escaping stack block a global block isa, that would allow the blocks runtime to avoid doing extra work when a non-escaping block is spuriously copied, and it would allow the compiler to completely avoid emitting copy and destroy helpers for the block. Please clear this with Greg Parker first, though.
• Even if that's not possible, you can still avoid emitting copy and destroy helpers in the common case where none of the captures are address-sensitive, i.e. __weak references or non-trivally-copyable C++ types, because the memcpy from the original block will be sufficient for correctness.
• You can just capture a reference to an outer block instead of copying anything that it captures.
• block variables which are only captured by non-escaping blocks are themselves known not to escape. (But remember that a non-escaping block can create an escaping block that captures the block variable!) Because block variable copies are only ever kicked off by block copy helpers, and you won't be generating those (or at least won't be asking them to copy your block variables for you), you also know that such variables can't be copied. That in turn means you can completely drop all the nonsense that only exists to supporting the lazy copying of block variables, like the block header and copy/destroy helpers and forwarding. Just generate the variable like a normal local variable and capture it by reference.

John.

include/clang/AST/Type.h

3156 ↗

(On Diff #106735)

Rather than adding an enum to ExtParameterInfo and modifying the type system, I made changes in IRGen to find out whether a function parameter is annotated with noescape. I'm not sure when it is OK or not OK to change the type system when adding support for an attribute, but this seemed to be the right direction since most of the other attributes are handled this way.

lib/CodeGen/CGCall.cpp

2096 ↗

(On Diff #106735)

Should we issue a warning when a parameter of an ObjC method is annotated with noescape and the corresponding parameter of the overriding method in a derived class isn't? Also, should we warn about inconsistent C++ virtual functions too?

[ahatanak]

Address review comments.

[rjmccall]

In D32210#819577, @ahatanak wrote:

In D32210#810508, @rjmccall wrote:

Hmm. Unfortunately, I'm not sure that's valid. The retains and releases of block captures don't protect against anything related to escaping the block; they protect against the original variables being modified during the lifetime of the block. It is true that a block literal passed to a noescape parameter has a shorter effective lifetime — we know for certain that it will be unused after the call, and ARC only promises that a value in an imprecise-lifetime strong variable like a block capture will be valid until the last load of that value from the variable. But that duration is still long enough for someone to modify the original variable in a way that is properly sequenced after the formation of the block.

Now, if you could prove that the variable was not modified for the duration of the call, that would be sufficient. And that would be easy to do in the common case by just proving that the address of the variable never escapes. Unfortunately, we don't have that information readily available because Sema doesn't collect it.

OK, so I guess the optimization isn't valid in the following case, for example:

void foo3(id);

id __strong *gp;

void foo2(id a) {
  gp = &a;
  nonescapingFunc(^{ foo3(a); }); // this function can modify "a" before the block is executed.
}

Right.

There are some other ways you could optimize blocks that are known not to escape, though. One big caveat is that you have to make sure the block behaves reasonably in response to being copied, becase being noescape doesn't guarantee that the callee won't try to copy the block. However:

I didn't understand the following optimizations. I thought clang doesn't emit copy and destroy helper for global blocks with or without noescape?

They're not really about global blocks, they're about whether we can emit a non-escaping block using the isa that we use for global blocks, which causes copies to be no-ops (because real global blocks do not have local captures).

John.

include/clang/AST/Type.h

3156 ↗

(On Diff #106735)

So, I do think it's acceptable to change the type system here; it's just that doing so definitely makes the change more complicated.

The guidance here is:

• If it changes the basic rules for making a call, it really *must* be preserved as part of the function type, or else basic things like taking the address of the function will never work correctly. That's why everything currently in ExtParameterInfo is there — ns_consumed affects the balancing rules for how ARC makes calls, the Swift parameter-ABI treatments are part of the CC, etc.
• Otherwise, it really comes down to how much we care about the feature working with indirection. I feel like blocks are probably the strongest motivator here because they're always invoked indirectly: if you want the feature to work on a block parameter, it really needs to be part of the function type, because we do not want to get into the business of finding non-canonical sources of information for function types, e.g. param decls from the TypeSourceInfo.

Overall, I would say that it tends to be the case that we eventually find attribute-based analyses limiting. If we start doing serious optimizations based on noescape, it is probably something that ought to go in the type system.

lib/CodeGen/CGCall.cpp

2096 ↗

(On Diff #106735)

Mmm. Probably yes in both cases, although we could alternatively just silently propagate the attribute InheritableParamAttr-style.

lib/CodeGen/CGClass.cpp

2086 ↗

(On Diff #107993)

Typo.

[ahatanak]

Include noescape attribute information in the function prototype and modify the AST printer and name mangling accordingly.

Also, make changes to allow implicit conversion from a function that takes noescape parameters to a function that doesn't. For example:

void (*fnptr0)(int *) = &noescapefunc; valid conversion.
void (*fnptr1)(attribute((noescape)) int *) = &escapefunc; invalid conversion.

lib/CodeGen/CGCall.cpp

2096 ↗

(On Diff #106735)

In the new patch, I chose to print diagnostics when the overriding method is missing noescape on its parameters lest users unintentionally let pointers escape.

[rjmccall]

include/clang/Basic/AttrDocs.td

138 ↗

(On Diff #109814)

Additionally, when the parameter is a `block pointer
<https://clang.llvm.org/docs/BlockLanguageSpec.html>`, the same restriction applies
to copies of the block.  For example:

include/clang/Sema/Sema.h

1480 ↗

(On Diff #109814)

I assume this only returns noescape from parameters that are not noescape in ToType.

lib/AST/ItaniumMangle.cpp

2700 ↗

(On Diff #109814)

This is not the right place to do this:

• It needs to be mangled in function types, not just on function declarations. It looks like the code here supporting pass_object_size just gets this wrong.
• We don't want to mangle this in top-level declarations because doing so will make it more directly ABI-breaking to retroactively add this attribute to a declaration.

Instead, you should handle it in mangleExtParameterInfo. Because these qualifiers are mangled in reverse alphabetical order, and "noescape" precedes "ns_consumed", it should be the last thing tested for in that function.

lib/Sema/SemaDeclCXX.cpp

14024 ↗

(On Diff #109814)

I think we generally encourage the uses of braces when the nested statement is this complex, even if it is technically a single statement.

lib/Sema/SemaDeclObjC.cpp

186 ↗

(On Diff #109814)

I know this is existing code, but you should probably at least warn about ns_consumed mismatches here in non-ARC.

lib/Sema/SemaExpr.cpp

7180 ↗

(On Diff #109814)

So this function removes noescape attributes from FromType to match ToType, but only if it's a legal conversion? Maybe it would be more reasonable to report an inability to convert in a more direct way, like by returning a null type.

Also, I think you're much more likely to avoid implicitly stripping sugar if you take both of these (or at least FromType?) as a QualType, and also return a QualType. Or you could pass the original QualType down separately and just return that in the cases where you don't need to change anything.

7251 ↗

(On Diff #109814)

I think the right place to do this is probably mergeFunctionTypes.

lib/Sema/SemaTemplate.cpp

5477 ↗

(On Diff #109814)

I think this is not the right place to be doing this; instead, we should be applying a conversion to the argument expression in CheckTemplateArgument. It looks like we already do that in C++17 mode, because C++17 broadened the rules here to allow an arbitrary function conversion (among the other things allowed by the "converted constant expression" rules, [temp.arg.nontype]p2). We probably could just leave it at that, since earlier standards are (in their infinite wisdom) stricter about type-matching here. If that poses a serious compatibility problem, the right fix is to modify the clause in CheckTemplateArgument for non-type template parameters where it deals with pointers, references, and member pointers to functions to specifically allow this conversion. But I'll note that we don't seem to be similarly generous about noreturn.

[ahatanak]

Address review comments.

lib/AST/ItaniumMangle.cpp

2700 ↗

(On Diff #109814)

Oh that's right. We should avoid breaking ABI when retroactively adding noescape.

lib/Sema/SemaDeclCXX.cpp

14024 ↗

(On Diff #109814)

I assume you are suggesting using braces for the if statement, not the for loop?

lib/Sema/SemaDeclObjC.cpp

186 ↗

(On Diff #109814)

I made warn_nsconsumed_attribute_mismatch and warn_nsreturns_retained_attribute_mismatch warnings that get promoted to errors when ARC is enabled.

lib/Sema/SemaExpr.cpp

7180 ↗

(On Diff #109814)

Yes. that's correct. The original implementation of the function removed noescape attributes from FromType's parameters to match ToType, but only if doing so made it a legal conversion.

I made changes to have this function take and return QualType and return QualType() when noescape on parameters makes the conversion invalid.

7251 ↗

(On Diff #109814)

When we have the following conditional operator expression, what is the type of the expression?

void func0(__attribute__((noescape)) int *a, int *b);
void func1(int *a, __attribute__((noescape)) int *b);

c ? func0 : func1;

The standard says the type of each parameter in the composite parameter type list is the composite type of the corresponding parameters of both functions, so I guess mergeFunctionType should drop noescape from both parameters?

lib/Sema/SemaTemplate.cpp

5477 ↗

(On Diff #109814)

I removed this code so that noescape is handled the same way noreturn is handled, which causes clang to complain about "S4<&noescapeFunc2> e1;" in test/SemaObjCXX/noescape.mm when it is not compiling for c++17 or later.

Is that what you meant?

[rjmccall]

lib/Sema/SemaDeclCXX.cpp

14024 ↗

(On Diff #109814)

Yes, exactly. This looks better, thank you.

lib/Sema/SemaExpr.cpp

7251 ↗

(On Diff #109814)

Yes, that sounds right.

7199 ↗

(On Diff #110612)

getAs<>, please.

7210 ↗

(On Diff #110612)

There's a potential for an illegal conversion here if ToProto->hasExtParameterInfos().

[ahatanak]

Address review comments.

The new patch defines a new function "mergeExtParameterInfo" that merges two functions' ExtParameterInfo lists and uses it where previously doFunctionTypesMatchOnExtParameterInfos was used.

lib/Sema/SemaExpr.cpp

7251 ↗

(On Diff #109814)

I defined a new function (mergeExtParameterInfo) that checks the compatibility of two functions and creates the composite type and used it for C's compatibility checking and C++'s conversion rules. Also, I added new test cases in test/Sema/noescape.c.

[rjmccall]

Thanks, that looks good.

[ahatanak]

Reopening this review. I accidentally closed this review when I committed r313717 (the patch that adds support for 'noescape').

[ahatanak]

This one can be closed.

cfe/trunk/include/clang/AST/ASTContext.h

QualType mergeFunctionParameterTypes(QualType, QualType,
                                        bool OfBlockPointer = false,
                                        bool Unqualified = false);
   QualType mergeTransparentUnionType(QualType, QualType,
                                      bool OfBlockPointer=false,
                                      bool Unqualified = false);
 
   QualType mergeObjCGCQualifiers(QualType, QualType);
 
-  bool doFunctionTypesMatchOnExtParameterInfos(
-         const FunctionProtoType *FromFunctionType,
-         const FunctionProtoType *ToFunctionType);
+  /// This function merges the ExtParameterInfo lists of two functions. It
+  /// returns true if the lists are compatible. The merged list is returned in
+  /// NewParamInfos.
+  ///
+  /// \param FirstFnType The type of the first function.
+  ///
+  /// \param SecondFnType The type of the second function.
+  ///
+  /// \param CanUseFirst This flag is set to true if the first function's
+  /// ExtParameterInfo list can be used as the composite list of
+  /// ExtParameterInfo.
+  ///
+  /// \param CanUseSecond This flag is set to true if the second function's
+  /// ExtParameterInfo list can be used as the composite list of
+  /// ExtParameterInfo.
+  ///
+  /// \param NewParamInfos The composite list of ExtParameterInfo. The list is
+  /// empty if none of the flags are set.
+  ///
+  bool mergeExtParameterInfo(
+      const FunctionProtoType *FirstFnType,
+      const FunctionProtoType *SecondFnType,
+      bool &CanUseFirst, bool &CanUseSecond,
+      SmallVectorImpl<FunctionProtoType::ExtParameterInfo> &NewParamInfos);
 
   void ResetObjCLayout(const ObjCContainerDecl *CD);
 
   //===--------------------------------------------------------------------===//
   //                    Integer Predicates
   //===--------------------------------------------------------------------===//
 
   // The width of an integer, as defined in C99 6.2.6.2. This is the number

cfe/trunk/include/clang/AST/Type.h

public:
   ///   void (*)(__attribute__((ns_consumed)) id)
   /// because the substitution of T=void, U=id into the former will
   /// not produce the latter.
   class ExtParameterInfo {
     enum {
       ABIMask         = 0x0F,
       IsConsumed      = 0x10,
       HasPassObjSize  = 0x20,
+      IsNoEscape      = 0x40,
     };
     unsigned char Data;
 
   public:
     ExtParameterInfo() : Data(0) {}
 
     /// Return the ABI treatment of this parameter.
     ParameterABI getABI() const {
bool hasPassObjectSize() const {
       return Data & HasPassObjSize;
     }
     ExtParameterInfo withHasPassObjectSize() const {
       ExtParameterInfo Copy = *this;
       Copy.Data |= HasPassObjSize;
       return Copy;
     }
 
+    bool isNoEscape() const {
+      return Data & IsNoEscape;
+    }
+
+    ExtParameterInfo withIsNoEscape(bool NoEscape) const {
+      ExtParameterInfo Copy = *this;
+      if (NoEscape)
+        Copy.Data |= IsNoEscape;
+      else
+        Copy.Data &= ~IsNoEscape;
+      return Copy;
+    }
+
     unsigned char getOpaqueValue() const { return Data; }
     static ExtParameterInfo getFromOpaqueValue(unsigned char data) {
       ExtParameterInfo result;
       result.Data = data;
       return result;
     }
 
     friend bool operator==(ExtParameterInfo lhs, ExtParameterInfo rhs) {

cfe/trunk/include/clang/Basic/Attr.td

def TypeNullUnspecified : TypeAttr {
   let Documentation = [TypeNullUnspecifiedDocs];
 }
 
 def ObjCKindOf : TypeAttr {
   let Spellings = [Keyword<"__kindof">];
   let Documentation = [Undocumented];
 }
 
+def NoEscape : Attr {
+  let Spellings = [GNU<"noescape">, CXX11<"clang", "noescape">];
+  let Subjects = SubjectList<[ParmVar]>;
+  let Documentation = [NoEscapeDocs];
+}
+
 def AssumeAligned : InheritableAttr {
   let Spellings = [GCC<"assume_aligned">];
   let Subjects = SubjectList<[ObjCMethod, Function]>;
   let Args = [ExprArgument<"Alignment">, ExprArgument<"Offset", 1>];
   let Documentation = [AssumeAlignedDocs];
 }
 
 def AllocAlign : InheritableAttr {

cfe/trunk/include/clang/Basic/AttrDocs.td

 In Clang, ``__declspec(thread)`` is generally equivalent in functionality to the
 GNU ``__thread`` keyword.  The variable must not have a destructor and must have
 a constant initializer, if any.  The attribute only applies to variables
 declared with static storage duration, such as globals, class static data
 members, and static locals.
   }];
 }
 
+def NoEscapeDocs : Documentation {
+  let Category = DocCatVariable;
+  let Content = [{
+``noescape`` placed on a function parameter of a pointer type is used to inform
+the compiler that the pointer cannot escape: that is, no reference to the object
+the pointer points to that is derived from the parameter value will survive
+after the function returns. Users are responsible for making sure parameters
+annotated with ``noescape`` do not actuallly escape.
+
+For example:
+
+.. code-block:: c
+  int *gp;
+
+  void nonescapingFunc(__attribute__((noescape)) int *p) {
+    *p += 100; // OK.
+  }
+
+  void escapingFunc(__attribute__((noescape)) int *p) {
+    gp = p; // Not OK.
+  }
+
+Additionally, when the parameter is a `block pointer
+<https://clang.llvm.org/docs/BlockLanguageSpec.html>`, the same restriction
+applies to copies of the block. For example:
+
+  typedef void (^BlockTy)();
+  BlockTy g0, g1;
+
+  void nonescapingFunc(__attribute__((noescape)) BlockTy block) {
+    block(); // OK.
+  }
+
+  void escapingFunc(__attribute__((noescape)) BlockTy block) {
+    g0 = block; // Not OK.
+    g1 = Block_copy(block); // Not OK either.
+  }
+
+  }];
+}
+
 def CarriesDependencyDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 The ``carries_dependency`` attribute specifies dependency propagation into and
 out of functions.
 
 When specified on a function or Objective-C method, the ``carries_dependency``
 attribute means that the return value carries a dependency out of the function,

cfe/trunk/include/clang/Basic/DiagnosticGroups.td

 def GNUConditionalOmittedOperand : DiagGroup<"gnu-conditional-omitted-operand">;
 def ConfigMacros : DiagGroup<"config-macros">;
 def : DiagGroup<"ctor-dtor-privacy">;
 def GNUDesignator : DiagGroup<"gnu-designator">;
 def GNUStringLiteralOperatorTemplate :
   DiagGroup<"gnu-string-literal-operator-template">;
 def UndefinedVarTemplate : DiagGroup<"undefined-var-template">;
 def UndefinedFuncTemplate : DiagGroup<"undefined-func-template">;
+def MissingNoEscape : DiagGroup<"missing-noescape">;
 
 def DeleteIncomplete : DiagGroup<"delete-incomplete">;
 def DeleteNonVirtualDtor : DiagGroup<"delete-non-virtual-dtor">;
 def AbstractFinalClass : DiagGroup<"abstract-final-class">;
 
 def CXX11CompatDeprecatedWritableStr :
   DiagGroup<"c++11-compat-deprecated-writable-strings">;
 

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def err_different_return_type_for_overriding_virtual_function : Error<
   "%diff{($) than the function it overrides (which has return type $)|"
   "than the function it overrides}1,2">;
 def note_overridden_virtual_function : Note<
   "overridden virtual function is here">;
 def err_conflicting_overriding_cc_attributes : Error<
   "virtual function %0 has different calling convention attributes "
   "%diff{($) than the function it overrides (which has calling convention $)|"
   "than the function it overrides}1,2">;
+def warn_overriding_method_missing_noescape : Warning<
+  "parameter of overriding method should be annotated with "
+  "__attribute__((noescape))">, InGroup<MissingNoEscape>;
+def note_overridden_marked_noescape : Note<
+  "parameter of overridden method is annotated with __attribute__((noescape))">;
 
 def err_covariant_return_inaccessible_base : Error<
   "invalid covariant return for virtual function: %1 is a "
   "%select{private|protected}2 base class of %0">, AccessControl;
 def err_covariant_return_ambiguous_derived_to_base_conv : Error<
   "return type of virtual function %3 is not covariant with the return type of "
   "the function it overrides (ambiguous conversion from derived class "
   "%0 to base class %1:%2)">;

cfe/trunk/lib/AST/ASTContext.cpp

if (lproto && rproto) { // two C99 style function prototypes
 
     // Variadic and non-variadic functions aren't compatible
     if (lproto->isVariadic() != rproto->isVariadic())
       return QualType();
 
     if (lproto->getTypeQuals() != rproto->getTypeQuals())
       return QualType();
 
-    if (!doFunctionTypesMatchOnExtParameterInfos(rproto, lproto))
+    SmallVector<FunctionProtoType::ExtParameterInfo, 4> newParamInfos;
+    bool canUseLeft, canUseRight;
+    if (!mergeExtParameterInfo(lproto, rproto, canUseLeft, canUseRight,
+                               newParamInfos))
       return QualType();
 
+    if (!canUseLeft)
+      allLTypes = false;
+    if (!canUseRight)
+      allRTypes = false;
+
     // Check parameter type compatibility
     SmallVector<QualType, 10> types;
     for (unsigned i = 0, n = lproto->getNumParams(); i < n; i++) {
       QualType lParamType = lproto->getParamType(i).getUnqualifiedType();
       QualType rParamType = rproto->getParamType(i).getUnqualifiedType();
       QualType paramType = mergeFunctionParameterTypes(
           lParamType, rParamType, OfBlockPointer, Unqualified);
       if (paramType.isNull())
for (unsigned i = 0, n = lproto->getNumParams(); i < n; i++) {
         allRTypes = false;
     }
       
     if (allLTypes) return lhs;
     if (allRTypes) return rhs;
 
     FunctionProtoType::ExtProtoInfo EPI = lproto->getExtProtoInfo();
     EPI.ExtInfo = einfo;
+    EPI.ExtParameterInfos =
+        newParamInfos.empty() ? nullptr : newParamInfos.data();
     return getFunctionType(retType, types, EPI);
   }
 
   if (lproto) allRTypes = false;
   if (rproto) allLTypes = false;
 
   const FunctionProtoType *proto = lproto ? lproto : rproto;
   if (proto) {
assert(LHS != RHS &&
            "Equivalent pipe types should have already been handled!");
     return QualType();
   }
   }
 
   llvm_unreachable("Invalid Type::Class!");
 }
 
-bool ASTContext::doFunctionTypesMatchOnExtParameterInfos(
-                   const FunctionProtoType *firstFnType,
-                   const FunctionProtoType *secondFnType) {
+bool ASTContext::mergeExtParameterInfo(
+    const FunctionProtoType *FirstFnType, const FunctionProtoType *SecondFnType,
+    bool &CanUseFirst, bool &CanUseSecond,
+    SmallVectorImpl<FunctionProtoType::ExtParameterInfo> &NewParamInfos) {
+  assert(NewParamInfos.empty() && "param info list not empty");
+  CanUseFirst = CanUseSecond = true;
+  bool FirstHasInfo = FirstFnType->hasExtParameterInfos();
+  bool SecondHasInfo = SecondFnType->hasExtParameterInfos();
+
   // Fast path: if the first type doesn't have ext parameter infos,
-  // we match if and only if they second type also doesn't have them.
-  if (!firstFnType->hasExtParameterInfos())
-    return !secondFnType->hasExtParameterInfos();
+  // we match if and only if the second type also doesn't have them.
+  if (!FirstHasInfo && !SecondHasInfo)
+    return true;
 
-  // Otherwise, we can only match if the second type has them.
-  if (!secondFnType->hasExtParameterInfos())
-    return false;
+  bool NeedParamInfo = false;
+  size_t E = FirstHasInfo ? FirstFnType->getExtParameterInfos().size()
+                          : SecondFnType->getExtParameterInfos().size();
+
+  for (size_t I = 0; I < E; ++I) {
+    FunctionProtoType::ExtParameterInfo FirstParam, SecondParam;
+    if (FirstHasInfo)
+      FirstParam = FirstFnType->getExtParameterInfo(I);
+    if (SecondHasInfo)
+      SecondParam = SecondFnType->getExtParameterInfo(I);
+
+    // Cannot merge unless everything except the noescape flag matches.
+    if (FirstParam.withIsNoEscape(false) != SecondParam.withIsNoEscape(false))
+      return false;
+
+    bool FirstNoEscape = FirstParam.isNoEscape();
+    bool SecondNoEscape = SecondParam.isNoEscape();
+    bool IsNoEscape = FirstNoEscape && SecondNoEscape;
+    NewParamInfos.push_back(FirstParam.withIsNoEscape(IsNoEscape));
+    if (NewParamInfos.back().getOpaqueValue())
+      NeedParamInfo = true;
+    if (FirstNoEscape != IsNoEscape)
+      CanUseFirst = false;
+    if (SecondNoEscape != IsNoEscape)
+      CanUseSecond = false;
+  }
 
-  auto firstEPI = firstFnType->getExtParameterInfos();
-  auto secondEPI = secondFnType->getExtParameterInfos();
-  assert(firstEPI.size() == secondEPI.size());
+  if (!NeedParamInfo)
+    NewParamInfos.clear();
 
-  for (size_t i = 0, n = firstEPI.size(); i != n; ++i) {
-    if (firstEPI[i] != secondEPI[i])
-      return false;
-  }
   return true;
 }
 
 void ASTContext::ResetObjCLayout(const ObjCContainerDecl *CD) {
   ObjCLayouts[CD] = nullptr;
 }
 
 /// mergeObjCGCQualifiers - This routine merges ObjC's GC attribute of 'LHS' and

cfe/trunk/lib/AST/ItaniumMangle.cpp

CXXNameMangler::mangleExtParameterInfo(FunctionProtoType::ExtParameterInfo PI) {
   case ParameterABI::SwiftErrorResult:
   case ParameterABI::SwiftIndirectResult:
     mangleVendorQualifier(getParameterABISpelling(PI.getABI()));
     break;
   }
 
   if (PI.isConsumed())
     mangleVendorQualifier("ns_consumed");
+
+  if (PI.isNoEscape())
+    mangleVendorQualifier("noescape");
 }
 
 // <type>          ::= <function-type>
 // <function-type> ::= [<CV-qualifiers>] F [Y]
 //                      <bare-function-type> [<ref-qualifier>] E
 void CXXNameMangler::mangleType(const FunctionProtoType *T) {
   mangleExtFunctionInfo(T);
 

cfe/trunk/lib/AST/TypePrinter.cpp

void TypePrinter::printFunctionProtoAfter(const FunctionProtoType *T,
   OS << '(';
   {
     ParamPolicyRAII ParamPolicy(Policy);
     for (unsigned i = 0, e = T->getNumParams(); i != e; ++i) {
       if (i) OS << ", ";
 
       auto EPI = T->getExtParameterInfo(i);
       if (EPI.isConsumed()) OS << "__attribute__((ns_consumed)) ";
+      if (EPI.isNoEscape())
+        OS << "__attribute__((noescape)) ";
       auto ABI = EPI.getABI();
       if (ABI != ParameterABI::Ordinary)
         OS << "__attribute__((" << getParameterABISpelling(ABI) << ")) ";
 
       print(T->getParamType(i), OS, StringRef());
     }
   }
   

cfe/trunk/lib/CodeGen/CGCall.cpp

 /// using the given receiver type.  The receiver type is not always
 /// the 'self' type of the method or even an Objective-C pointer type.
 /// This is *not* the right method for actually performing such a
 /// message send, due to the possibility of optional arguments.
 const CGFunctionInfo &
 CodeGenTypes::arrangeObjCMessageSendSignature(const ObjCMethodDecl *MD,
                                               QualType receiverType) {
   SmallVector<CanQualType, 16> argTys;
+  SmallVector<FunctionProtoType::ExtParameterInfo, 4> extParamInfos(2);
   argTys.push_back(Context.getCanonicalParamType(receiverType));
   argTys.push_back(Context.getCanonicalParamType(Context.getObjCSelType()));
   // FIXME: Kill copy?
   for (const auto *I : MD->parameters()) {
     argTys.push_back(Context.getCanonicalParamType(I->getType()));
+    auto extParamInfo = FunctionProtoType::ExtParameterInfo().withIsNoEscape(
+        I->hasAttr<NoEscapeAttr>());
+    extParamInfos.push_back(extParamInfo);
   }
 
   FunctionType::ExtInfo einfo;
   bool IsWindows = getContext().getTargetInfo().getTriple().isOSWindows();
   einfo = einfo.withCallingConv(getCallingConventionForDecl(MD, IsWindows));
 
   if (getContext().getLangOpts().ObjCAutoRefCount &&
       MD->hasAttr<NSReturnsRetainedAttr>())
     einfo = einfo.withProducesResult(true);
 
   RequiredArgs required =
     (MD->isVariadic() ? RequiredArgs(argTys.size()) : RequiredArgs::All);
 
   return arrangeLLVMFunctionInfo(
       GetReturnType(MD->getReturnType()), /*instanceMethod=*/false,
-      /*chainCall=*/false, argTys, einfo, {}, required);
+      /*chainCall=*/false, argTys, einfo, extParamInfos, required);
 }
 
 const CGFunctionInfo &
 CodeGenTypes::arrangeUnprototypedObjCMessageSend(QualType returnType,
                                                  const CallArgList &args) {
   auto argTypes = getArgTypesForCall(Context, args);
   FunctionType::ExtInfo einfo;
 
case ParameterABI::SwiftErrorResult:
       Attrs.addAttribute(llvm::Attribute::SwiftError);
       break;
 
     case ParameterABI::SwiftContext:
       Attrs.addAttribute(llvm::Attribute::SwiftSelf);
       break;
     }
 
+    if (FI.getExtParameterInfo(ArgNo).isNoEscape())
+      Attrs.addAttribute(llvm::Attribute::NoCapture);
+
     if (Attrs.hasAttributes()) {
       unsigned FirstIRArg, NumIRArgs;
       std::tie(FirstIRArg, NumIRArgs) = IRFunctionArgs.getIRArgs(ArgNo);
       for (unsigned i = 0; i < NumIRArgs; i++)
         ArgAttrs[FirstIRArg + i] =
             llvm::AttributeSet::get(getLLVMContext(), Attrs);
     }
   }

cfe/trunk/lib/Sema/SemaDeclAttr.cpp

if (!attrNonNullArgCheck(S, ResultType, Attr, SourceRange(), SR,
                            /* isReturnValue */ true))
     return;
 
   D->addAttr(::new (S.Context)
             ReturnsNonNullAttr(Attr.getRange(), S.Context,
                                Attr.getAttributeSpellingListIndex()));
 }
 
+static void handleNoEscapeAttr(Sema &S, Decl *D, const AttributeList &Attr) {
+  if (D->isInvalidDecl())
+    return;
+
+  // noescape only applies to pointer types.
+  QualType T = cast<ParmVarDecl>(D)->getType();
+  if (!S.isValidPointerAttrType(T, /* RefOkay */ true)) {
+    S.Diag(Attr.getLoc(), diag::warn_attribute_pointers_only)
+        << Attr.getName() << Attr.getRange() << 0;
+    return;
+  }
+
+  D->addAttr(::new (S.Context) NoEscapeAttr(
+      Attr.getRange(), S.Context, Attr.getAttributeSpellingListIndex()));
+}
+
 static void handleAssumeAlignedAttr(Sema &S, Decl *D,
                                     const AttributeList &Attr) {
   Expr *E = Attr.getArgAsExpr(0),
        *OE = Attr.getNumArgs() > 1 ? Attr.getArgAsExpr(1) : nullptr;
   S.AddAssumeAlignedAttr(Attr.getRange(), D, E, OE,
                          Attr.getAttributeSpellingListIndex());
 }
 
case AttributeList::AT_NonNull:
     if (ParmVarDecl *PVD = dyn_cast<ParmVarDecl>(D))
       handleNonNullAttrParameter(S, PVD, Attr);
     else
       handleNonNullAttr(S, D, Attr);
     break;
   case AttributeList::AT_ReturnsNonNull:
     handleReturnsNonNullAttr(S, D, Attr);
     break;
+  case AttributeList::AT_NoEscape:
+    handleNoEscapeAttr(S, D, Attr);
+    break;
   case AttributeList::AT_AssumeAligned:
     handleAssumeAlignedAttr(S, D, Attr);
     break;
   case AttributeList::AT_AllocAlign:
     handleAllocAlignAttr(S, D, Attr);
     break;
   case AttributeList::AT_Overloadable:
     handleSimpleAttribute<OverloadableAttr>(S, D, Attr);

cfe/trunk/lib/Sema/SemaDeclCXX.cpp

void Sema::DiagnoseReturnInConstructorExceptionHandler(CXXTryStmt *TryBlock) {
   for (unsigned I = 0, E = TryBlock->getNumHandlers(); I != E; ++I) {
     CXXCatchStmt *Handler = TryBlock->getHandler(I);
     SearchForReturnInStmt(*this, Handler);
   }
 }
 
 bool Sema::CheckOverridingFunctionAttributes(const CXXMethodDecl *New,
                                              const CXXMethodDecl *Old) {
-  const FunctionType *NewFT = New->getType()->getAs<FunctionType>();
-  const FunctionType *OldFT = Old->getType()->getAs<FunctionType>();
+  const auto *NewFT = New->getType()->getAs<FunctionProtoType>();
+  const auto *OldFT = Old->getType()->getAs<FunctionProtoType>();
+
+  if (OldFT->hasExtParameterInfos()) {
+    for (unsigned I = 0, E = OldFT->getNumParams(); I != E; ++I)
+      // A parameter of the overriding method should be annotated with noescape
+      // if the corresponding parameter of the overridden method is annotated.
+      if (OldFT->getExtParameterInfo(I).isNoEscape() &&
+          !NewFT->getExtParameterInfo(I).isNoEscape()) {
+        Diag(New->getParamDecl(I)->getLocation(),
+             diag::warn_overriding_method_missing_noescape);
+        Diag(Old->getParamDecl(I)->getLocation(),
+             diag::note_overridden_marked_noescape);
+      }
+  }
 
   CallingConv NewCC = NewFT->getCallConv(), OldCC = OldFT->getCallConv();
 
   // If the calling conventions match, everything is fine
   if (NewCC == OldCC)
     return false;
 
   // If the calling conventions mismatch because the new function is static,

cfe/trunk/lib/Sema/SemaDeclObjC.cpp

for (ObjCMethodDecl::param_iterator ni = NewMethod->param_begin(),
        ni != ne && oi != oe; ++ni, ++oi) {
     const ParmVarDecl *oldDecl = (*oi);
     ParmVarDecl *newDecl = (*ni);
     if (newDecl->hasAttr<NSConsumedAttr>() !=
         oldDecl->hasAttr<NSConsumedAttr>()) {
       Diag(newDecl->getLocation(), diag::warn_nsconsumed_attribute_mismatch);
       Diag(oldDecl->getLocation(), diag::note_previous_decl) << "parameter";
     }
+
+    // A parameter of the overriding method should be annotated with noescape
+    // if the corresponding parameter of the overridden method is annotated.
+    if (oldDecl->hasAttr<NoEscapeAttr>() && !newDecl->hasAttr<NoEscapeAttr>()) {
+      Diag(newDecl->getLocation(),
+           diag::warn_overriding_method_missing_noescape);
+      Diag(oldDecl->getLocation(), diag::note_overridden_marked_noescape);
+    }
   }
 }
 
 /// \brief Check a method declaration for compatibility with the Objective-C
 /// ARC conventions.
 bool Sema::CheckARCMethodDecl(ObjCMethodDecl *method) {
   ObjCMethodFamily family = method->getMethodFamily();
   switch (family) {

cfe/trunk/lib/Sema/SemaOverload.cpp

if (FromFPT->isNothrow(Context) && !ToFPT->isNothrow(Context)) {
       FromFn = cast<FunctionType>(
           Context.getFunctionType(FromFPT->getReturnType(),
                                   FromFPT->getParamTypes(),
                                   FromFPT->getExtProtoInfo().withExceptionSpec(
                                       FunctionProtoType::ExceptionSpecInfo()))
                  .getTypePtr());
       Changed = true;
     }
+
+    // Convert FromFPT's ExtParameterInfo if necessary. The conversion is valid
+    // only if the ExtParameterInfo lists of the two function prototypes can be
+    // merged and the merged list is identical to ToFPT's ExtParameterInfo list.
+    SmallVector<FunctionProtoType::ExtParameterInfo, 4> NewParamInfos;
+    bool CanUseToFPT, CanUseFromFPT;
+    if (Context.mergeExtParameterInfo(ToFPT, FromFPT, CanUseToFPT,
+                                      CanUseFromFPT, NewParamInfos) &&
+        CanUseToFPT && !CanUseFromFPT) {
+      FunctionProtoType::ExtProtoInfo ExtInfo = FromFPT->getExtProtoInfo();
+      ExtInfo.ExtParameterInfos =
+          NewParamInfos.empty() ? nullptr : NewParamInfos.data();
+      QualType QT = Context.getFunctionType(FromFPT->getReturnType(),
+                                            FromFPT->getParamTypes(), ExtInfo);
+      FromFn = QT->getAs<FunctionType>();
+      Changed = true;
+    }
   }
 
   if (!Changed)
     return false;
 
   assert(QualType(FromFn, 0).isCanonical());
   if (QualType(FromFn, 0) != CanTo) return false;
 
if (Context.hasSameType(FromArgType, ToArgType)) {
                                         ConvertedType, IncompatibleObjC)) {
        if (IncompatibleObjC)
          return false;
        // Okay, we have an Objective-C pointer conversion.
      } else
        // Argument types are too different. Abort.
        return false;
    }
-   if (!Context.doFunctionTypesMatchOnExtParameterInfos(FromFunctionType,
-                                                        ToFunctionType))
+
+   SmallVector<FunctionProtoType::ExtParameterInfo, 4> NewParamInfos;
+   bool CanUseToFPT, CanUseFromFPT;
+   if (!Context.mergeExtParameterInfo(ToFunctionType, FromFunctionType,
+                                      CanUseToFPT, CanUseFromFPT,
+                                      NewParamInfos))
      return false;
 
    ConvertedType = ToType;
    return true;
 }
 
 enum {
   ft_default,

cfe/trunk/lib/Sema/SemaType.cpp

case DeclaratorChunk::Function: {
             HasAnyInterestingExtParameterInfos = true;
           }
 
           if (Param->hasAttr<PassObjectSizeAttr>()) {
             ExtParameterInfos[i] = ExtParameterInfos[i].withHasPassObjectSize();
             HasAnyInterestingExtParameterInfos = true;
           }
 
+          if (Param->hasAttr<NoEscapeAttr>()) {
+            ExtParameterInfos[i] = ExtParameterInfos[i].withIsNoEscape(true);
+            HasAnyInterestingExtParameterInfos = true;
+          }
+
           ParamTys.push_back(ParamTy);
         }
 
         if (HasAnyInterestingExtParameterInfos) {
           EPI.ExtParameterInfos = ExtParameterInfos.data();
           checkExtParameterInfos(S, ParamTys, EPI,
               [&](unsigned i) { return FTI.Params[i].Param->getLocation(); });
         }

cfe/trunk/test/CodeGenCXX/noescape.cpp

+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+struct S {
+  int a[4];
+  S(int *, int * __attribute__((noescape)));
+  S &operator=(int * __attribute__((noescape)));
+  void m0(int *, int * __attribute__((noescape)));
+  virtual void vm1(int *, int * __attribute__((noescape)));
+};
+
+// CHECK: define void @_ZN1SC2EPiS0_(%struct.S* {{.*}}, {{.*}}, {{.*}} nocapture)
+// CHECK: define void @_ZN1SC1EPiS0_(%struct.S* {{.*}}, {{.*}}, {{.*}} nocapture) {{.*}} {
+// CHECK: call void @_ZN1SC2EPiS0_(%struct.S* {{.*}}, {{.*}}, {{.*}} nocapture {{.*}})
+
+S::S(int *, int * __attribute__((noescape))) {}
+
+// CHECK: define {{.*}} %struct.S* @_ZN1SaSEPi(%struct.S* {{.*}}, {{.*}} nocapture)
+S &S::operator=(int * __attribute__((noescape))) { return *this; }
+
+// CHECK: define void @_ZN1S2m0EPiS0_(%struct.S* {{.*}}, {{.*}} nocapture)
+void S::m0(int *, int * __attribute__((noescape))) {}
+
+// CHECK: define void @_ZN1S3vm1EPiS0_(%struct.S* {{.*}}, {{.*}} nocapture)
+void S::vm1(int *, int * __attribute__((noescape))) {}
+
+// CHECK-LABEL: define void @_Z5test0P1SPiS1_(
+// CHECK: call void @_ZN1SC1EPiS0_(%struct.S* {{.*}}, {{.*}}, {{.*}} nocapture {{.*}})
+// CHECK: call {{.*}} %struct.S* @_ZN1SaSEPi(%struct.S* {{.*}}, {{.*}} nocapture {{.*}})
+// CHECK: call void @_ZN1S2m0EPiS0_(%struct.S* {{.*}}, {{.*}}, {{.*}} nocapture {{.*}})
+// CHECK: call void {{.*}}(%struct.S* {{.*}}, {{.*}}, {{.*}} nocapture {{.*}})
+void test0(S *s, int *p0, int *p1) {
+  S t(p0, p1);
+  t = p1;
+  s->m0(p0, p1);
+  s->vm1(p0, p1);
+}
+
+namespace std {
+  typedef decltype(sizeof(0)) size_t;
+}
+
+// CHECK: define {{.*}} @_ZnwmPv({{.*}}, {{.*}} nocapture {{.*}})
+void *operator new(std::size_t, void * __attribute__((noescape)) p) {
+  return p;
+}
+
+// CHECK-LABEL: define i8* @_Z5test1Pv(
+// CHECK : %call = call {{.*}} @_ZnwmPv({{.*}}, {{.*}} nocapture {{.*}})
+void *test1(void *p0) {
+  return ::operator new(16, p0);
+}
+
+// CHECK-LABEL: define void @_Z5test2PiS_(
+// CHECK: call void @"_ZZ5test2PiS_ENK3$_0clES_S_"({{.*}}, {{.*}}, {{.*}} nocapture {{.*}})
+// CHECK: define internal void @"_ZZ5test2PiS_ENK3$_0clES_S_"({{.*}}, {{.*}}, {{.*}} nocapture)
+void test2(int *p0, int *p1) {
+  auto t = [](int *, int * __attribute__((noescape))){};
+  t(p0, p1);
+}
+
+// CHECK-LABEL: define void @_Z5test3PFvU8noescapePiES_(
+// CHECK: call void {{.*}}(i32* nocapture {{.*}})
+typedef void (*NoEscapeFunc)(__attribute__((noescape)) int *);
+
+void test3(NoEscapeFunc f, int *p) {
+  f(p);
+}

cfe/trunk/test/CodeGenObjC/noescape.m

+// RUN: %clang_cc1 -fblocks -emit-llvm -o - %s | FileCheck %s
+
+typedef void (^BlockTy)(void);
+
+union U {
+  int *i;
+  long long *ll;
+} __attribute__((transparent_union));
+
+void noescapeFunc0(id, __attribute__((noescape)) BlockTy);
+void noescapeFunc1(__attribute__((noescape)) int *);
+void noescapeFunc2(__attribute__((noescape)) id);
+void noescapeFunc3(__attribute__((noescape)) union U);
+
+// CHECK-LABEL: define void @test0(
+// CHECK: call void @noescapeFunc0({{.*}}, {{.*}} nocapture {{.*}})
+// CHECK: declare void @noescapeFunc0(i8*, {{.*}} nocapture)
+void test0(BlockTy b) {
+  noescapeFunc0(0, b);
+}
+
+// CHECK-LABEL: define void @test1(
+// CHECK: call void @noescapeFunc1({{.*}} nocapture {{.*}})
+// CHECK: declare void @noescapeFunc1({{.*}} nocapture)
+void test1(int *i) {
+  noescapeFunc1(i);
+}
+
+// CHECK-LABEL: define void @test2(
+// CHECK: call void @noescapeFunc2({{.*}} nocapture {{.*}})
+// CHECK: declare void @noescapeFunc2({{.*}} nocapture)
+void test2(id i) {
+  noescapeFunc2(i);
+}
+
+// CHECK-LABEL: define void @test3(
+// CHECK: call void @noescapeFunc3({{.*}} nocapture {{.*}})
+// CHECK: declare void @noescapeFunc3({{.*}} nocapture)
+void test3(union U u) {
+  noescapeFunc3(u);
+}
+
+// CHECK: define internal void @"\01-[C0 m0:]"({{.*}}, {{.*}}, {{.*}} nocapture {{.*}})
+
+// CHECK-LABEL: define void @test4(
+// CHECK: call void bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to void (i8*, i8*, i32*)*)(i8* {{.*}}, i8* {{.*}}, i32* nocapture {{.*}})
+
+@interface C0
+-(void) m0:(int*)__attribute__((noescape)) p0;
+@end
+
+@implementation C0
+-(void) m0:(int*)__attribute__((noescape)) p0 {
+}
+@end
+
+void test4(C0 *c0, int *p) {
+  [c0 m0:p];
+}
+
+// CHECK-LABEL: define void @test5(
+// CHECK: call void {{.*}}(i8* bitcast ({ i8**, i32, i32, i8*, {{.*}} }* @{{.*}} to i8*), i32* nocapture {{.*}})
+// CHECK: call void {{.*}}(i8* {{.*}}, i32* nocapture {{.*}})
+// CHECK: define internal void @{{.*}}(i8* {{.*}}, i32* nocapture {{.*}})
+
+typedef void (^BlockTy2)(__attribute__((noescape)) int *);
+
+void test5(BlockTy2 b, int *p) {
+  ^(int *__attribute__((noescape)) p0){}(p);
+  b(p);
+}

cfe/trunk/test/Misc/ast-dump-attr.cpp

 // CHECK: FunctionDecl{{.*}} TestExternalSourceSymbolAttr4
 // CHECK-NEXT: ExternalSourceSymbolAttr{{.*}} "C Sharp" "Some external file.cs" GeneratedDeclaration
 
 void TestExternalSourceSymbolAttr5()
 __attribute__((external_source_symbol(generated_declaration, defined_in="module", language="Swift")));
 // CHECK: FunctionDecl{{.*}} TestExternalSourceSymbolAttr5
 // CHECK-NEXT: ExternalSourceSymbolAttr{{.*}} "Swift" "module" GeneratedDeclaration
 
+namespace TestNoEscape {
+  void noescapeFunc(int *p0, __attribute__((noescape)) int *p1) {}
+  // CHECK: `-FunctionDecl{{.*}} noescapeFunc 'void (int *, __attribute__((noescape)) int *)'
+  // CHECK-NEXT: ParmVarDecl
+  // CHECK-NEXT: ParmVarDecl
+  // CHECK-NEXT: NoEscapeAttr
+}
+
 namespace TestSuppress {
   [[gsl::suppress("at-namespace")]];
   // CHECK: NamespaceDecl{{.*}} TestSuppress
   // CHECK-NEXT: EmptyDecl{{.*}}
   // CHECK-NEXT: SuppressAttr{{.*}} at-namespace
   [[gsl::suppress("on-decl")]]
   void TestSuppressFunction();
   // CHECK: FunctionDecl{{.*}} TestSuppressFunction

cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test

 // RUN: clang-tblgen -gen-clang-test-pragma-attribute-supported-attributes -I%src_include_dir %src_include_dir/clang/Basic/Attr.td -o - | FileCheck %s
 
 // The number of supported attributes should never go down!
 
-// CHECK: #pragma clang attribute supports 64 attributes:
+// CHECK: #pragma clang attribute supports 65 attributes:
 // CHECK-NEXT: AMDGPUFlatWorkGroupSize (SubjectMatchRule_function)
 // CHECK-NEXT: AMDGPUNumSGPR (SubjectMatchRule_function)
 // CHECK-NEXT: AMDGPUNumVGPR (SubjectMatchRule_function)
 // CHECK-NEXT: AMDGPUWavesPerEU (SubjectMatchRule_function)
 // CHECK-NEXT: AVRSignal (SubjectMatchRule_function)
 // CHECK-NEXT: AbiTag (SubjectMatchRule_record_not_is_union, SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_namespace)
 // CHECK-NEXT: AlignValue (SubjectMatchRule_variable, SubjectMatchRule_type_alias)
 // CHECK-NEXT: AllocSize (SubjectMatchRule_function)

 // CHECK-NEXT: IFunc (SubjectMatchRule_function)
 // CHECK-NEXT: InternalLinkage (SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_record)
 // CHECK-NEXT: LTOVisibilityPublic (SubjectMatchRule_record)
 // CHECK-NEXT: MicroMips (SubjectMatchRule_function)
 // CHECK-NEXT: MipsLongCall (SubjectMatchRule_function)
 // CHECK-NEXT: MipsShortCall (SubjectMatchRule_function)
 // CHECK-NEXT: NoDebug (SubjectMatchRule_hasType_functionType, SubjectMatchRule_objc_method, SubjectMatchRule_variable_not_is_parameter)
 // CHECK-NEXT: NoDuplicate (SubjectMatchRule_function)
+// CHECK-NEXT: NoEscape (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)
 // CHECK-NEXT: NoSanitize (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSanitizeSpecific (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSplitStack (SubjectMatchRule_function)
 // CHECK-NEXT: NotTailCalled (SubjectMatchRule_function)
 // CHECK-NEXT: ObjCBoxable (SubjectMatchRule_record)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)

cfe/trunk/test/Sema/noescape.c

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void escapefunc(int *);
+void noescapefunc(__attribute__((noescape)) int *);
+void (*escapefuncptr)(int *);
+void (*noescapefuncptr)(__attribute__((noescape)) int *);
+
+void func_ne(__attribute__((noescape)) int *, int *);
+void func_en(int *, __attribute__((noescape)) int *);
+
+void (*funcptr_ee)(int *, int *);
+void (*funcptr_nn)(__attribute__((noescape)) int *, __attribute__((noescape)) int *);
+
+void test0(int c) {
+  escapefuncptr = &escapefunc;
+  escapefuncptr = &noescapefunc;
+  noescapefuncptr = &escapefunc; // expected-warning {{incompatible function pointer types assigning to 'void (*)(__attribute__((noescape)) int *)' from 'void (*)(int *)'}}
+  noescapefuncptr = &noescapefunc;
+
+  escapefuncptr = c ? &escapefunc : &noescapefunc;
+  noescapefuncptr = c ? &escapefunc : &noescapefunc; // expected-warning {{incompatible function pointer types assigning to 'void (*)(__attribute__((noescape)) int *)' from 'void (*)(int *)'}}
+
+  funcptr_ee = c ? &func_ne : &func_en;
+  funcptr_nn = c ? &func_ne : &func_en; // expected-warning {{incompatible function pointer types assigning to 'void (*)(__attribute__((noescape)) int *, __attribute__((noescape)) int *)' from 'void (*)(int *, int *)'}}
+}

cfe/trunk/test/SemaObjCXX/noescape.mm

+// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -std=c++1z %s
+
+typedef void (^BlockTy)();
+
+struct S {
+  int i;
+  void m();
+};
+
+void noescapeFunc0(id, __attribute__((noescape)) BlockTy);
+void noescapeFunc1(id, [[clang::noescape]] BlockTy);
+void noescapeFunc2(__attribute__((noescape)) int *); // expected-note {{previous declaration is here}}
+void noescapeFunc3(__attribute__((noescape)) id);
+void noescapeFunc4(__attribute__((noescape)) int &);
+void noescapeFunc2(int *); // expected-error {{conflicting types for 'noescapeFunc2'}}
+
+void invalidFunc0(int __attribute__((noescape))); // expected-warning {{'noescape' attribute only applies to pointer arguments}}
+void invalidFunc1(int __attribute__((noescape(0)))); // expected-error {{'noescape' attribute takes no arguments}}
+void invalidFunc2(int0 *__attribute__((noescape))); // expected-error {{use of undeclared identifier 'int0'; did you mean 'int'?}}
+void invalidFunc3(__attribute__((noescape)) int (S::*Ty)); // expected-warning {{'noescape' attribute only applies to pointer arguments}}
+void invalidFunc4(__attribute__((noescape)) void (S::*Ty)()); // expected-warning {{'noescape' attribute only applies to pointer arguments}}
+int __attribute__((noescape)) g; // expected-warning {{'noescape' attribute only applies to parameters}}
+
+struct S1 {
+  virtual void m0(int *__attribute__((noescape))); // expected-note {{parameter of overridden method is annotated with __attribute__((noescape))}}
+};
+
+struct S2 : S1 {
+  void m0(int *__attribute__((noescape))) override;
+};
+
+struct S3 : S1 {
+  void m0(int *) override; // expected-warning {{parameter of overriding method should be annotated with __attribute__((noescape))}}
+};
+
+__attribute__((objc_root_class))
+@interface C0
+-(void) m0:(int*)__attribute__((noescape)) p; // expected-note {{parameter of overridden method is annotated with __attribute__((noescape))}}
+@end
+
+@implementation C0
+-(void) m0:(int*)__attribute__((noescape)) p {}
+@end
+
+@interface C1 : C0
+-(void) m0:(int*)__attribute__((noescape)) p;
+@end
+
+@implementation C1 : C0
+-(void) m0:(int*)__attribute__((noescape)) p {}
+@end
+
+@interface C2 : C0
+-(void) m0:(int*) p; // expected-warning {{parameter of overriding method should be annotated with __attribute__((noescape))}}
+@end
+
+@implementation C2 : C0
+-(void) m0:(int*) p {}
+@end
+
+void func0(int *);
+void (*fnptr0)(int *);
+void (*fnptr1)(__attribute__((noescape)) int *);
+template<void (*fn)(int*)> struct S4 {};
+template<void (*fn)(int* __attribute__((noescape)))> struct S5 {};
+
+#if __cplusplus < 201406
+  // expected-note@-4 {{template parameter is declared here}}
+  // expected-note@-4 {{template parameter is declared here}}
+#endif
+
+void test0() {
+  fnptr0 = &func0;
+  fnptr0 = &noescapeFunc2;
+  fnptr1 = &func0; // expected-error {{assigning to 'void (*)(__attribute__((noescape)) int *)' from incompatible type 'void (*)(int *)'}}
+  fnptr1 = &noescapeFunc2;
+  S4<&func0> e0;
+  S4<&noescapeFunc2> e1;
+  S5<&func0> ne0;
+
+#if __cplusplus < 201406
+  // expected-error@-4 {{non-type template argument of type 'void (*)(__attribute__((noescape)) int *)' cannot be converted to a value of type 'void (*)(int *)'}}
+  // expected-error@-4 {{non-type template argument of type 'void (*)(int *)' cannot be converted to a value of type 'void (*)(__attribute__((noescape)) int *)'}}
+#else
+  // expected-error@-6 {{value of type 'void (*)(int *)' is not implicitly convertible to 'void (*)(__attribute__((noescape)) int *)'}}
+#endif
+
+  S5<&noescapeFunc2> ne1;
+}