john.brawn marked 4 inline comments as done.
john.brawn added inline comments.
================
Comment at: clang/lib/Sema/ParsedAttr.cpp:143
+ // otherwise return a default ParsedAttrInfo.
+ const ParsedAttrInfo *Info = AttrInfoMap[A.getKind()];
+ if (Info)
----------------
erichkeane wrote:
> 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.
AttrInfoMap is declared to have size ParsedAttr::UnknownAttribute+1, so when
the kind is too large we get an element that wasn't explicitly initialized so
will be null.
Doing it by kind makes more sense though, so I'll do that.
================
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:
> 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.
================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:3637
+ SS << "ParsedAttrInfo" << I->first;
+ SS << " parsedAttrInfo" << I->first << "Instance;\n\n";
}
----------------
erichkeane wrote:
> Should the instance be static? Perhaps a static variable in the class itself?
>
Making it static makes sense, I'll do that.
================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:3641
- OS << "static const ParsedAttrInfo AttrInfoMap[ParsedAttr::UnknownAttribute "
+ OS << "static const ParsedAttrInfo *AttrInfoMap[ParsedAttr::UnknownAttribute
"
"+ 1] = {\n";
----------------
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).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D31337/new/
https://reviews.llvm.org/D31337
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
