[PATCH] D31337: Use virtual functions in ParsedAttrInfo instead of function pointers

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/Sema/ParsedAttr.cpp:143
+  // otherwise return a default ParsedAttrInfo.
+  const ParsedAttrInfo *Info = AttrInfoMap[A.getKind()];
+  if (Info)
----------------
I don't think you can do this.  The only way to get Info as nullptr is to end up off the end of the array, which is UB.

Instead of the map lookup, this should check A.getKind against the range.


================
Comment at: clang/lib/Sema/ParsedAttr.cpp:146
+    return *Info;
+  return DefaultParsedAttrInfo;
 }
----------------
Currently the behavior of this is to make an invalid attribute kind be UB.  Presumably the point here is to make it so that is no longer the case?


================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:3400
 
-static std::string GenerateAppertainsTo(const Record &Attr, raw_ostream &OS) {
+static void GenerateAppertainsTo(const Record &Attr, raw_ostream &SS,
+                                 raw_ostream &OS) {
----------------
Why does this take SS AND OS.  What is the difference here?  Does this actually use OS anymore?


================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:3637
+    SS << "ParsedAttrInfo" << I->first;
+    SS << " parsedAttrInfo" << I->first << "Instance;\n\n";
   }
----------------
Should the instance be static? Perhaps a static variable in the class itself?  


================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:3641
 
-  OS << "static const ParsedAttrInfo AttrInfoMap[ParsedAttr::UnknownAttribute "
+  OS << "static const ParsedAttrInfo *AttrInfoMap[ParsedAttr::UnknownAttribute "
         "+ 1] = {\n";
----------------
Is there a good reason this doesn't return references?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D31337/new/

https://reviews.llvm.org/D31337