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:
> > 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?


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

Reply via email to