aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Sema/ParsedAttr.h:45
+struct ParsedAttrInfo {
+ /// Corresponds to the Kind enum
+ unsigned AttrKind : 16;
----------------
Please add a full stop to the end of all the comments (here and elsewhere).
================
Comment at: clang/include/clang/Sema/ParsedAttr.h:63
+ unsigned IsSupportedByPragmaAttribute : 1;
+ // The syntaxes supported by this attribute and how they're spelt
+ struct Spelling {
----------------
spelt -> spelled
================
Comment at: clang/include/clang/Sema/ParsedAttr.h:615
AttributeCommonInfo::Kind getKind() const { return getParsedKind(); }
+ ParsedAttrInfo &getInfo() const { return *Info; }
};
----------------
I think this should return a `const ParsedAttrInfo&` so that callers don't try
to mutate it.
================
Comment at: clang/lib/Basic/Attributes.cpp:101-103
+ for (ParsedAttrInfoRegistry::iterator it = ParsedAttrInfoRegistry::begin(),
+ ie = ParsedAttrInfoRegistry::end();
+ it != ie; ++it) {
----------------
Range-based for loop? Also, `it` and `ie` don't meet the usual naming
conventions.
One thing I'm not keen on with this is the performance hit. We spent a decent
amount of effort making this lookup fast and it's now a linear search through
all of the attributes and requires memory allocations and deallocations to
perform the search.
================
Comment at: clang/lib/Basic/Attributes.cpp:113
+ // If we failed to find a match then the attribute is unknown
+ return std::make_unique<ParsedAttrInfo>();
+}
----------------
This seems surprising because it makes it harder to determine whether you have
a valid result from this function or not. I think this should return a null
`unique_ptr`.
================
Comment at: clang/lib/Basic/Attributes.cpp:117
+std::unique_ptr<ParsedAttrInfo>
+ParsedAttrInfo::get(AttributeCommonInfo::Kind K) {
+ // Search for a ParsedAttrInfo whose kind matches
----------------
Same concerns in this function as above.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D31338/new/
https://reviews.llvm.org/D31338
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
