Bug 500870 - NS_OVERRIDE indicates that a method must override a base-class method, r=taras
authorBenjamin Smedberg <benjamin@smedbergs.us>
Fri, 04 Sep 2009 11:21:31 -0400
changeset 32868 8150d919e280be1b3bc4ae87d12ba865f98f5e36
parent 32867 d1f1ad39e10e4f263db7d956cfcd02d917fef70c
child 32869 8b60809857c36d08c25831ff530a59ac59e489d2
child 35930 d1e8ebf88247191f19da7af9c75af66133c64380
push id9203
push userbsmedberg@mozilla.com
push dateFri, 18 Sep 2009 18:55:34 +0000
treeherdermozilla-central@8150d919e280 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstaras
bugs500870
milestone1.9.3a1pre
Bug 500870 - NS_OVERRIDE indicates that a method must override a base-class method, r=taras
config/static-checking-config.mk
config/static-checking.js
xpcom/analysis/override.js
xpcom/base/nscore.h
xpcom/tests/static-checker/Makefile.in
xpcom/tests/static-checker/override-global.cpp
xpcom/tests/static-checker/override-pass.cpp
xpcom/tests/static-checker/override-signature.cpp
xpcom/tests/static-checker/override-static.cpp
xpcom/tests/static-checker/override-virtual.cpp
--- a/config/static-checking-config.mk
+++ b/config/static-checking-config.mk
@@ -1,15 +1,16 @@
 # The entire tree should be subject to static analysis using the XPCOM
 # script. Additional scripts may be added by specific subdirectories.
 
 DEHYDRA_SCRIPT = $(topsrcdir)/config/static-checking.js
 
 DEHYDRA_MODULES = \
   $(topsrcdir)/xpcom/analysis/final.js \
+  $(topsrcdir)/xpcom/analysis/override.js \
   $(topsrcdir)/xpcom/analysis/must-override.js \
   $(NULL)
 
 TREEHYDRA_MODULES = \
   $(topsrcdir)/xpcom/analysis/outparams.js \
   $(topsrcdir)/xpcom/analysis/stack.js \
   $(topsrcdir)/xpcom/analysis/flow.js \
   $(topsrcdir)/js/src/jsstack.js \
--- a/config/static-checking.js
+++ b/config/static-checking.js
@@ -57,16 +57,22 @@ function hasAttribute(c, attrname)
   return false;
 }
 
 // This is useful for detecting method overrides
 function signaturesMatch(m1, m2)
 {
   if (m1.shortName != m2.shortName)
     return false;
+
+  if (m1.isVirtual != m2.isVirtual)
+    return false;
+  
+  if (m1.isStatic != m2.isStatic)
+    return false;
   
   let p1 = m1.type.parameters;
   let p2 = m2.type.parameters;
   
   if (p1.length != p2.length)
     return false;
   
   for (let i = 0; i < p1.length; ++i)
new file mode 100644
--- /dev/null
+++ b/xpcom/analysis/override.js
@@ -0,0 +1,42 @@
+/**
+ * NS_OVERRIDE may be marked on class methods which are intended to override
+ * a method in a base class. If the method is removed or altered in the base
+ * class, the compiler will force all subclass overrides to be modified.
+ */
+
+/**
+ * Generate all the base classes recursively of class `c`.
+ */
+function all_bases(c)
+{
+  for each (let b in c.bases) {
+    yield b.type;
+    for (let bb in all_bases(b.type))
+      yield bb;
+  }
+}
+
+function process_decl(d)
+{
+  if (!hasAttribute(d, 'NS_override'))
+    return;
+
+  if (!d.memberOf || !d.isFunction) {
+    error("%s is marked NS_OVERRIDE but is not a class function.".format(d.name), d.loc);
+    return;
+  }
+
+  if (d.isStatic) {
+    error("Marking NS_OVERRIDE on static function %s is meaningless.".format(d.name), d.loc);
+    return;
+  }
+
+  for (let base in all_bases(d.memberOf)) {
+    for each (let m in base.members) {
+      if (m.shortName == d.shortName && signaturesMatch(m, d))
+	return;
+    }
+  }
+
+  error("NS_OVERRIDE function %s does not override a base class method with the same name and signature".format(d.name), d.loc);
+}
--- a/xpcom/base/nscore.h
+++ b/xpcom/base/nscore.h
@@ -500,16 +500,18 @@ typedef PRUint32 nsrefcnt;
 /**
  * Attributes defined to help Dehydra GCC analysis.
  */
 #ifdef NS_STATIC_CHECKING
 # define NS_SCRIPTABLE __attribute__((user("NS_script")))
 # define NS_INPARAM __attribute__((user("NS_inparam")))
 # define NS_OUTPARAM  __attribute__((user("NS_outparam")))
 # define NS_INOUTPARAM __attribute__((user("NS_inoutparam")))
+# define NS_OVERRIDE __attribute__((user("NS_override")))
 #else
 # define NS_SCRIPTABLE
 # define NS_INPARAM
 # define NS_OUTPARAM
 # define NS_INOUTPARAM
+# define NS_OVERRIDE
 #endif
 
 #endif /* nscore_h___ */
--- a/xpcom/tests/static-checker/Makefile.in
+++ b/xpcom/tests/static-checker/Makefile.in
@@ -119,33 +119,46 @@ MUST_OVERRIDE_PASS_TESTCASES = \
 
 MUST_OVERRIDE_FAILURE_TESTCASES = \
   OverrideFail1.cpp \
   OverrideFail2.cpp \
   OverrideFail3.cpp \
   OverrideFail4.cpp \
   $(NULL)
 
+OVERRIDE_PASS_TESTCASES = \
+  override-pass.cpp \
+  $(NULL)
+
+OVERRIDE_FAILURE_TESTCASES = \
+  override-global.cpp \
+  override-signature.cpp \
+  override-static.cpp \
+  override-virtual.cpp \
+  $(NULL)
+
 STATIC_FAILURE_TESTCASES = \
   $(FINAL_FAILURE_TESTCASES) \
   $(FLOW_FAILURE_TESTCASES) \
   $(MUST_OVERRIDE_FAILURE_TESTCASES) \
+  $(OVERRIDE_FAILURE_TESTCASES) \
   $(NULL)
 
 STATIC_WARNING_TESTCASES = \
   $(OUTPARAMS_WARNING_TESTCASES) \
   $(STACK_FAILURE_TESTCASES) \
   $(NULL)
 
 STATIC_PASS_TESTCASES = \
   $(OUTPARAMS_NS_FAILED_TESTCASES) \
   $(OUTPARAMS_PASS_TESTCASES) \
   $(STACK_PASS_TESTCASES) \
   $(FLOW_PASS_TESTCASES) \
   $(MUST_OVERRIDE_PASS_TESTCASES) \
+  $(OVERRIDE_PASS_TESTCASES) \
   $(NULL)
 
 
 include $(topsrcdir)/config/rules.mk
 
 # We want to compile each file and invert the result to ensure that
 # compilation failed.
 check:: \
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/static-checker/override-global.cpp
@@ -0,0 +1,1 @@
+__attribute__((user("NS_override"))) int m();
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/static-checker/override-pass.cpp
@@ -0,0 +1,15 @@
+class A
+{
+  int a(int, char*);
+  void c(char);
+};
+
+class B : A
+{
+  __attribute__((user("NS_override"))) int a(int, char*);
+};
+
+class C : B
+{
+  __attribute__((user("NS_override"))) void c(char);
+};
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/static-checker/override-signature.cpp
@@ -0,0 +1,9 @@
+class A
+{
+  int m(int);
+};
+
+class B : A
+{
+  __attribute__((user("NS_override"))) int m(void*);
+};
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/static-checker/override-static.cpp
@@ -0,0 +1,9 @@
+class A
+{
+  static int m();
+};
+
+class B : A
+{
+  __attribute__((user("NS_override"))) static int m();
+};
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/static-checker/override-virtual.cpp
@@ -0,0 +1,9 @@
+class A
+{
+  int m();
+};
+
+class B : A
+{
+  __attribute__((user("NS_override"))) virtual int m();
+};