vsapsai marked 2 inline comments as done.
vsapsai added inline comments.
================
Comment at: clang/include/clang/Basic/SourceLocation.h:202
+/// Can be used transparently in places where SourceLocation is expected.
+class MultiSourceLocation {
+ bool IsSingleLoc;
----------------
erik.pilkington wrote:
> 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.
I've tried to go middle way this time and have `MultiSourceLocation` just as
typedef. I modeled it after `MultiExprArg` which is `using MultiExprArg =
MutableArrayRef<Expr *>;` But that approach can have different reasons and
isn't necessarily applicable in this case.
My problem here is that on one hand I think `MultiSourceLocation` might be a
useful abstraction and in that case probably `Sema::BuildInstanceMessage`
should use this type for its parameter. On the other hand I am struggling to
come up with good explanation what `MultiSourceLocation` is. And that's an
indication it's not a good abstraction. What do you think?
================
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) {
----------------
erik.pilkington wrote:
> 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.
I like "try" part. But not sure about "Replacement". This function doesn't care
how its output will be used so I don't think it is worth mentioning.
I've updated comment for `Name` but not entirely satisfied with the wording.
Will try to come up with something better and suggestions are welcome.
https://reviews.llvm.org/D44589
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
