[cfe-commits] [MS] Improved diagnostics of inheritance attributes

John McCall rjmccall at apple.com
Tue Jun 26 22:45:25 PDT 2012


On Jun 26, 2012, at 10:31 PM, João Matos wrote:
> Attached is a patch that adds the semantic checking to the previously
> added MS inheritance model attributes.


+def warn_ms_inheritance_already_declared : Warning<
+  "ignored since inheritance model was already declared as '"
+  "%select{single|multiple|virtual}0'">;

This should definitely be an error.  Even if this is a warning in MSVC, I want this to be an error unless it's literally blocking headers from compiling.

+static bool hasOtherInheritanceAttr(Decl *D, AttributeList::Kind Kind,
+    int& Existing) {
+  if (Kind != AttributeList::AT_SingleInheritance &&
+      D->hasAttr<SingleInheritanceAttr>()) {
+    Existing = 0;
+    return true;
+  }
+  else if (Kind != AttributeList::AT_MultipleInheritance &&
+      D->hasAttr<MultipleInheritanceAttr>()) {
+    Existing = 1;
+    return true;
+  }
+  else if (Kind != AttributeList::AT_VirtualInheritance &&
+      D->hasAttr<VirtualInheritanceAttr>()) {
+    Existing = 2;
+    return true;
+  }
+  return false;
+}

Hmm.  In retrospect, it would really be better to model this as a single Attr kind that carries an enum distinguishing the cases.  Would you mind?

Please also add code to validate this attribute vs. the actual inheritance properties of the class once it's defined.  You'll want to test both paths, i.e. both adding a definition to a previously-declared class and redeclaring a class that's already been defined.

The grammar prevents us from having multiple of these attributes on a single declaration, right?  We should at least have a test for that.

John.



More information about the cfe-commits mailing list