[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/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) {
----------------
john.brawn wrote:
> erichkeane wrote:
> > Why does this take SS AND OS.  What is the difference here?  Does this actually use OS anymore?
> It's because of GenerateCustomAppertainsTo.  That generates a function that expects to be at file scope (because emitAttributeMatchRules re-uses the same function), but at the time GenerateAppertainsTo is called SS is in the middle of the ParsedAttrInfo. Previously both the function that GenerateCustomAppertainsTo generates and that this file generates would be at file scope, so both were written to OS.
Thanks!


================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:3637
+    SS << "ParsedAttrInfo" << I->first;
+    SS << " parsedAttrInfo" << I->first << "Instance;\n\n";
   }
----------------
john.brawn wrote:
> erichkeane wrote:
> > Should the instance be static? Perhaps a static variable in the class itself?  
> Making it static makes sense, I'll do that.
static class member is what I'm thinking.  That way it is ParsedAttrInfoWHATEVER::Instance?


================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:3641
 
-  OS << "static const ParsedAttrInfo AttrInfoMap[ParsedAttr::UnknownAttribute "
+  OS << "static const ParsedAttrInfo *AttrInfoMap[ParsedAttr::UnknownAttribute "
         "+ 1] = {\n";
----------------
john.brawn wrote:
> erichkeane wrote:
> > Is there a good reason this doesn't return references?
> You can't have an array of references (C++11 dcl.ref paragraph 5).
Ah, right... got lost in the code-that-generates-code thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D31337