1307702 - Reject usage of MOZ_REQUIRED_BASE_METHOD on non-virtual base methods.

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect, P3)

Description

[Andi [:andi]]

This patch is a companion of this: https://bugzilla.mozilla.org/show_bug.cgi?id=1230311

It prevents the usage of MOZ_REQUIRED_BASE_METHOD annotation on non-virtual functions.

Comment 1

[Andi [:andi]]

Attachment: Bug 1307702 - clang-plugin - static analysis to reject ussage of MOZ_REQUIRED_BASE_METHOD on non-virtual base methods.

Review commit: https://reviewboard.mozilla.org/r/83520/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83520/

Comment 2

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

Comment on attachment 8797939 [details]
Bug 1307702 - clang-plugin - static analysis to reject ussage of MOZ_REQUIRED_BASE_METHOD on non-virtual base methods.

https://reviewboard.mozilla.org/r/83520/#review82218

Looks good! Can we land this after we land the OverrideBaseCallChecker? I feel like it would be confusing to have this code in the tree without that code present.

::: build/clang-plugin/clang-plugin.cpp:888
(Diff revision 1)
> +  const CXXMethodDecl *Decl = Node.getCanonicalDecl();
> +  return Decl
> +      && MozChecker::hasCustomAnnotation(Decl, "moz_required_base_method");
> +}
> +
> +AST_MATCHER(CXXMethodDecl, isNotVirtual) {

Can we call this "NonVirtual"?

Comment 3

[Andi [:andi]]

Comment on attachment 8797939 [details]
Bug 1307702 - clang-plugin - static analysis to reject ussage of MOZ_REQUIRED_BASE_METHOD on non-virtual base methods.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/83520/diff/1-2/

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3429ce702ffd
clang-plugin - static analysis to reject ussage of MOZ_REQUIRED_BASE_METHOD on non-virtual base methods. r=mystor

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/3429ce702ffd