1554989 - Inheriting constructors whose inherited constructors are implicit are always rejected

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Yaron Tausky [:ytausky]]

The matcher checks the inherited constructors for implicitness but the introducing declaration for the MOZ_IMPLICIT annotation. Since a using-declaration cannot have attributes, such constructors are always rejected.

Comment 1

[Andi [:andi]]

Yaron it would really help if you could provide an example here.

Comment 2

[Yaron Tausky [:ytausky]]

Sure:

class A {
 public:
  MOZ_IMPLICIT A(int) {}
};

class B : public A {
 public:
  using A::A;
};

The using A::A; declaration introduces an implicit constructor, but there's no way to mark it as intended, as far as I can tell.

Comment 3

[Yaron Tausky [:ytausky]]

Attachment: Bug 1554989 - Fix implicit checker on inheriting constructors

Inheriting constructors are implicitly introduced via a using-declaration.
Since the C++ grammar doesn't allow attributes on using-declarations, it
is currently impossible to add MOZ_IMPLICIT to implicit inheriting
constructors.

This commit changes the AST matcher such that it ignores inheriting
constructors altogether. If they are inheriting from an implicit inherited
constructor, that constructor's check should be enough to ensure that no
constructors are unintentionally implicit.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f99b54a6da93
Fix implicit checker on inheriting constructors r=andi

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/f99b54a6da93

Comment 6

[Yaron Tausky [:ytausky]]

Attachment: Bug 1554989 - Fix implicit checker on inheriting constructors r=andi

Inheriting constructors are implicitly introduced via a using-declaration.
Since the C++ grammar doesn't allow attributes on using-declarations, it
is currently impossible to add MOZ_IMPLICIT to implicit inheriting
constructors.

This commit changes the AST matcher such that it ignores inheriting
constructors altogether. If they are inheriting from an implicit inherited
constructor, that constructor's check should be enough to ensure that no
constructors are unintentionally implicit.

Comment 7

[Yaron Tausky [:ytausky]]

Comment on attachment 9088121 [details]
Bug 1554989 - Fix implicit checker on inheriting constructors r=andi

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This bug fix is required in order to land another important patch.
• User impact if declined: Inability to land an important patch due to static analysis error.
• Fix Landed on Version: 69
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch only changes the static analysis check and none of the user-visible code.
• String or UUID changes made by this patch:

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9088121 [details]
Bug 1554989 - Fix implicit checker on inheriting constructors r=andi

Bustage fix for another patch being uplifted. Approved for 68.1esr.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/f635742506af

Comment 10

[Julien Cristau [:jcristau]]

Comment on attachment 9088121 [details]
Bug 1554989 - Fix implicit checker on inheriting constructors r=andi

this is also needed on esr60

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr60/rev/2db1de7836b82ebecbd96a4067b1976077a6394c