[PATCH] D18768: Refactoring attribute subject diagnostics

2017-12-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman abandoned this revision.
aaron.ballman added a comment.

Closing this via "abandon" so that Richard doesn't have to accept it in order 
for me to close it. It's already been committed.


https://reviews.llvm.org/D18768



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D18768: Refactoring attribute subject diagnostics

2017-11-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

I had the chance to finalize this long-standing review by fixing up the 
remaining test cases and removing the unused enumerators from AttributeList.h. 
I've commit in r319002 (but cannot close the review because the "Accept" didn't 
happen).


https://reviews.llvm.org/D18768



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D18768: Refactoring attribute subject diagnostics

2016-04-22 Thread Richard Smith via cfe-commits
rsmith added a comment.

I think this is a reasonable direction. I'd like to see some kind of %list form 
added to our diagnostics system, but I don't mind whether that happens before 
or after this change.

(I'd also prefer to use singular forms in the various subject names, but that 
seems to create problems with the presence/absence of "a" / "an". If you see a 
way to make that work, we should consider it, but otherwise don't worry about 
it.)


http://reviews.llvm.org/D18768



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D18768: Refactoring attribute subject diagnostics

2016-04-22 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment.

Ping


http://reviews.llvm.org/D18768



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D18768: Refactoring attribute subject diagnostics

2016-04-04 Thread Aaron Ballman via cfe-commits
aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, doug.gregor.
aaron.ballman added a subscriber: cfe-commits.

Currently, attribute subject lists attempt to automatically determine what 
diagnostic to emit when the attribute does not appertain to a given 
declaration. It does so by the attribute tablegen emitter looking at the 
subject list and attempting to match it up with a viable enumerator in 
AttributeList.h. This approach is starting to show signs of wear now that we 
are getting more complex attribute subject lists. It also runs into problems 
where users don't want to add a one-off enumerator and instead use an existing 
close-but-not-quite-accurate diagnostic.

This patch attempts to improve the situation by removing the need for the 
enumeration and complex selection logic in the diagnostics tablegen. Instead, 
each declaration and statement node tablegen description includes a 
comma-separated list of subjects for diagnostics that the attribute tablegen 
emitter uses to create a single string to pass to a new diagnostic. This 
approach is not ideal -- ideally we would have a "list" notion for the 
diagnostics engine that could handle properly formatting a list so that it 
handles multiple elements for us. However, I think my current approach is an 
incremental improvement that gets us a step closer to being able to use such a 
list mechanism were it to be implemented in the future.

Before I correct all of the test cases for minor diagnostic differences, I 
wanted to point out those differences explicitly:

* The following were renamed to reduce confusion:
  * methods -> Objective-C methods
  * interfaces -> Objective-C interfaces
  * protocols -> Objective-C protocols
  * fields -> non-static data members
  * types -> typedefs
* We used to consider CXXRecordDecl to only be "classes" while RecordDecl was 
"struct, union, or class"; we would drop the "or class" part when not compiling 
in C++ mode. Now that we've switched to a string literal, dropping (or adding) 
"classes" is more challenging and so RecordDecl always reports "classes" under 
the assumption that it won't be overly confusing to C users. I am also banking 
on there being more C++ users than C users, but an alternative may be to drop 
"classes" from RecordDecl under the assumption that users understand "struct" 
is the effectively same thing as a "class" in C++.

Note that the actual declarations the attribute appertains to haven't changed, 
just the spelling of the diagnostics.

If the general approach taken here is reasonable, then I will update the patch 
with corrected test cases and other polish-related fixes. But since it affects 
~30 test cases (due to the updated terminology, consistent use of 'and' instead 
of 'or', etc), I didn't want to go through the effort unless the patch is going 
in a reasonable direction.

http://reviews.llvm.org/D18768

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/DeclNodes.td
  include/clang/Basic/DiagnosticSemaKinds.td
  utils/TableGen/ClangAttrEmitter.cpp

Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -2419,120 +2419,72 @@
   OS << "}\n\n";
 }
 
+static std::string GetDiagnosticSpelling(const Record &R) {
+  std::string Ret = R.getValueAsString("DiagSpelling");
+  if (!Ret.empty())
+return Ret;
+
+  // If we couldn't find the DiagSpelling in this object, we can check to see
+  // if the object is one that has a base, and if it is, loop up to the Base
+  // member recursively.
+  std::string Super = R.getSuperClasses().back().first->getName();
+  if (Super == "DDecl" || Super == "DStmt")
+return GetDiagnosticSpelling(*R.getValueAsDef("Base"));
+
+  return "";
+}
+
 static std::string CalculateDiagnostic(const Record &S) {
   // If the SubjectList object has a custom diagnostic associated with it,
   // return that directly.
   std::string CustomDiag = S.getValueAsString("CustomDiag");
   if (!CustomDiag.empty())
-return CustomDiag;
+return '"' + CustomDiag + '"';
 
-  // Given the list of subjects, determine what diagnostic best fits.
-  enum {
-Func = 1U << 0,
-Var = 1U << 1,
-ObjCMethod = 1U << 2,
-Param = 1U << 3,
-Class = 1U << 4,
-GenericRecord = 1U << 5,
-Type = 1U << 6,
-ObjCIVar = 1U << 7,
-ObjCProp = 1U << 8,
-ObjCInterface = 1U << 9,
-Block = 1U << 10,
-Namespace = 1U << 11,
-Field = 1U << 12,
-CXXMethod = 1U << 13,
-ObjCProtocol = 1U << 14,
-Enum = 1U << 15
-  };
-  uint32_t SubMask = 0;
-
+  std::vector DiagList;
   std::vector Subjects = S.getValueAsListOfDefs("Subjects");
   for (const auto *Subject : Subjects) {
 const Record &R = *Subject;
-std::string Name;
-
-if (R.isSubClassOf("SubsetSubject")) {
-  PrintError(R.getLoc(), "SubsetSubjects should use a custom diagnostic");
-  // As a fallback, lo