1878108 - Consider using `[[clang:lifetime_bound]]` attribute for FunctionRef

Status: RESOLVED FIXED

(Core :: MFBT, enhancement)

Description

[Nika Layzell [:nika] (ooo, ni? for response)]

In bug 1877672, the FunctionRef type is made to be MOZ_TEMPORARY_CLASS to prevent it from being used outside of a temporary context. This is being done to avoid issues with a temporary lambda being passed into the FunctionRef, which is then kept around longer than the constructor's expression.

Out of curiosity I noticed that absl has a similar FunctionRef type (https://searchfox.org/mozilla-central/rev/5c4a45eb17423373ecb71aea9819d41a6231613e/third_party/libwebrtc/third_party/abseil-cpp/absl/functional/function_ref.h#104), which has neither mitigation we have used for our type, but specifies this ABSL_ATTRIBUTE_LIFETIME_BOUND annotation. This appears to be a clang-specific annotation for cases like the one in bug 1877672 (https://clang.llvm.org/docs/AttributeReference.html#lifetimebound).

Unlike our custom static analysis, builtin clang attributes will be checked on developer's machines if they build with clang, without the clang-plugin enabled, meaning that it may catch bugs earlier in development.

This bug is to track potentially adding attributes for this annotation to Attributes.h, and potentially using it for our FunctionRef type (either in addition to MOZ_TEMPORARY_CLASS or instead of that attribute).

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1878108 - Replace custom MOZ_LIFETIME_BOUND with built-in. r=nika,serge-sans-paille

The built-in version is better as it also allows annotating particular
parameters (it not only applies to method declarations).

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1878108 - Annotate FunctionRef constructor with MOZ_LIFETIME_BOUND. r=nika,serge-sans-paille

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bb356c39c0e2
Replace custom MOZ_LIFETIME_BOUND with built-in. r=nika,glandium
https://hg.mozilla.org/integration/autoland/rev/05e2d2bc1260
Annotate FunctionRef constructor with MOZ_LIFETIME_BOUND. r=nika

Comment 4

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/da4017ea7dfb
Backed out 2 changesets for causing sm bustages in Attributes.h. CLOSED TREE

Comment 5

[pstanciu]

Backed out for causing sm bustages in Attributes.h.

• Backout link
• Push with failures
• Failure Log
• Failure line:

[task 2024-02-06T10:10:59.162Z] /builds/worker/workspace/obj-spider/dist/include/mozilla/Attributes.h:457:32: error: missing ')' after "__has_attribute"
[task 2024-02-06T10:10:59.162Z]  #  if __has_cpp_attribute(clang::lifetimebound)
[task 2024-02-06T10:10:59.162Z]                                 ^
[task 2024-02-06T10:10:59.162Z] /builds/worker/workspace/obj-spider/dist/include/mozilla/Attributes.h:457:33: error:  ':' without preceding '?'
[task 2024-02-06T10:10:59.162Z]  #  if __has_cpp_attribute(clang::lifetimebound)
[task 2024-02-06T10:10:59.162Z]                                  ^
[task 2024-02-06T10:10:59.162Z] gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:600: pure_virtual.o] Error 1
[task 2024-02-06T10:10:59.162Z] gmake[4]: Leaving directory '/builds/worker/workspace/obj-spider/build/pure_virtual'
[task 2024-02-06T10:10:59.162Z] gmake[4]: Target 'target-objects' not remade because of errors.
[task 2024-02-06T10:10:59.162Z] gmake[3]: *** [/builds/worker/checkouts/gecko/config/recurse.mk:72: build/pure_virtual/target-objects] Error 2

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bad3848767f6
Replace custom MOZ_LIFETIME_BOUND with built-in. r=nika,glandium
https://hg.mozilla.org/integration/autoland/rev/d9f3cf9b2cb7
Annotate FunctionRef constructor with MOZ_LIFETIME_BOUND. r=nika

Comment 7

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/bad3848767f6
https://hg.mozilla.org/mozilla-central/rev/d9f3cf9b2cb7