compnerd marked 3 inline comments as done.
compnerd added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:104
+def ObjCClassMethod
+ : SubsetSubject<ObjCMethod, [{!S->isInstanceMethod()}],
----------------
aaron.ballman wrote:
> This change is no longer needed.
Ah, right, I'll move it to the `swift_private` change.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4289
+ if (Inline->getName() != Name && !Inline->isImplicit()) {
+ Diag(Inline->getLocation(), diag::warn_attribute_ignored) << Inline;
+ Diag(CI.getLoc(), diag::note_conflicting_attribute);
----------------
aaron.ballman wrote:
> compnerd wrote:
> > aaron.ballman wrote:
> > > I think it would be more helpful if the diagnostic said why the attribute
> > > is being ignored (because the arguments don't match).
> > Does the note below not accomplish that?
> Not really, no. The warning diagnostic itself just says that the attribute is
> ignored, which is the effect but not the rationale. The note (which is easier
> for folks to ignore) says the attribute is conflicting, but conflicting with
> *what* (there could be a half dozen attributes on the same declaration, for
> instance).
Okay, I don't think that there is an existing warning, but I should be able to
add one.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5804-5805
+ if (const auto *Method = dyn_cast<ObjCMethodDecl>(D)) {
+ ParamCount = Method->getSelector().getNumArgs();
+ Params = Method->parameters().slice(0, ParamCount);
+ } else {
----------------
aaron.ballman wrote:
> compnerd wrote:
> > aaron.ballman wrote:
> > > Do you have to worry about functions with `...` variadic parameters and
> > > how those impact counts (for either ObjC or regular methods)?
> > No, they are currently not auto-imported AFAIK.
> Should such function signatures be rejected here though? (If so, can you add
> a test for that case as well as a test using a K&R C declaration like `void
> f()`?)
I'll add a test case demonstrating that, it is indeed missing.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87534/new/
https://reviews.llvm.org/D87534
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
