Mordante marked 3 inline comments as done.
Mordante added inline comments.
================
Comment at: clang/lib/AST/Comment.cpp:151
+static bool getFunctionTypeLoc(TypeLoc TL, FunctionTypeLoc &ResFTL,
+ bool testTypedefTypeLoc = false) {
TypeLoc PrevTL;
----------------
gribozavr wrote:
> Mordante wrote:
> > gribozavr wrote:
> > > Why is the new functionality turned off sometimes? It seems to me that we
> > > always want to look through typedefs.
> > Setting `testTypedefTypeLoc` to `true` breaks a unit test in
> > `test/Sema/warn-documentation.cpp:358`
> >
> > ```
> > typedef int (*test_not_function_like_typedef1)(int aaa);
> >
> > // expected-warning@+1 {{'\param' command used in a comment that is not
> > attached to a function declaration}}
> > /// \param aaa Meow.
> >
> > typedef test_not_function_like_typedef1 test_not_function_like_typedef2;
> > ```
> > and its corresponding test using a `using` instead of `typedef`. This has
> > been introduced in:
> > ```
> > commit 49fdf8d3f5e114e6f8b49fde29a30b0eaaa3c5dd
> > Author: Dmitri Gribenko <griboz...@gmail.com>
> > Date: Sat Sep 15 21:13:36 2012 +0000
> >
> > Comment parsing: don't treat typedef to a typedef to a function as a
> > 'function-like' type that can be annotated with \param.
> >
> > Thanks to Eli Friedman for noticing!
> >
> > llvm-svn: 163985
> >
> > ```
> > I'm not sure whether or not we should allow this typedef documentation. I
> > just tested with Doxygen. It doesn't complain and shows the parameter
> > documentation for `test_not_function_like_typedef2`. So on that basis we
> > could remove this `expected-warning` and `testTypedefTypeLoc`.
> >
> > What do you think?
> Thanks for the explanation. I can't find the context for that decision, and
> the commit description does not explain the reason either.
>
> It is really a policy decision -- when introducing a comment for function
> return value and arguments, should the declaration include the said return
> value and arguments, or can they be visible through a typedef?
>
> Thinking about it, my inclination is to say that comments for the return
> value and arguments should be present on the declaration that introduces
> them. Otherwise, it is breaking an abstraction barrier.
>
> WDYT?
(I just noticed I forgot to submit this comment.)
I tend to agree. However Doxygen accepts the code, but Doxygen doesn't seem to
validate whether parameters exist.
So I wonder whether we do our users a service by warning about code that
Doxygen properly handles. On that basis I think we can allow it.
We could add an extra warning in the pedantic group and let the user decide
whether or not she thinks it warrants a warning.
I think that would be the best solution: accept the comment but allow the user
to opt-in on a warning.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66706/new/
https://reviews.llvm.org/D66706
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
