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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/ParsedAttr.cpp:141
 
+static ParsedAttrInfo DefaultParsedAttrInfo;
 static const ParsedAttrInfo &getInfo(const ParsedAttr &A) {
----------------
Might as well lower this variable into the function -- no real need for it to have file scope.


================
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) {
----------------
erichkeane wrote:
> 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!
I think the stream parameter names should be a bit more descriptive here (and perhaps some comments on the function would be a good idea as well). Perhaps `FileScopeStream` and `LexicalScopeStream`?


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

https://reviews.llvm.org/D31337