erik.pilkington added a comment.
Hi Volodymyr, thanks for working on this! Overall this looks good, I just have
a few nits.
================
Comment at: clang/include/clang/Basic/SourceLocation.h:202
+/// Can be used transparently in places where SourceLocation is expected.
+class MultiSourceLocation {
+ bool IsSingleLoc;
----------------
Why can't we just use an ArrayRef<SourceLocation> for this? It looks like that
type already has a converting constructor from SourceLocation, so we should be
able to use in in DiagnoseUseOfDecl without any noise.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6934-6938
+// Returns a number of method parameters if parsing is successful.
+// In case of unsuccessful parsing SlotNames can contain invalid data.
+static Optional<unsigned>
+parseObjCMethodName(StringRef Name, SmallVectorImpl<StringRef> &SlotNames,
+ const LangOptions &LangOpts) {
----------------
Maybe tryParseReplacementObjCMethodName or something would be better?
parseObjCMethodName() is pretty vague. Also the comment above should probably
be a `///` doxygen comment. It would be nice if you mentioned that `Name`
originated from an availability attribute in the comment.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6959-6966
+ if (S.empty())
+ continue;
+ if (!isIdentifierHead(S[0], AllowDollar))
+ return None;
+ for (char C : S)
+ if (!isIdentifierBody(C, AllowDollar))
+ return None;
----------------
Might be better to add a defaulted AllowDollar parameter to isValidIdentifier()
in CharInfo.h and use that rather than duplicating it here.
https://reviews.llvm.org/D44589
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
