gribozavr added inline comments.
================
Comment at: clang/lib/AST/Comment.cpp:151
+static bool getFunctionTypeLoc(TypeLoc TL, FunctionTypeLoc &ResFTL,
+ bool testTypedefTypeLoc = false) {
TypeLoc PrevTL;
----------------
Mordante wrote:
> 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.
Doxygen and C++ both allow a lot of dubious stuff. That can't be a reason to
avoid producing a warning -- we would have no warnings if that was the bar.
I think what we should discuss is, what is the use case for adding a `\returns`
command in this position -- and how is documenting parameters not a use case.
If there is a valid use case, how common it is in real world.
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
