[PATCH] D63276: [AST] Add FunctionDecl::getParametersSourceRange()

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/AST/Decl.h:2331
+  /// Attempt to compute an informative source range covering the
+  /// function parameters. This omits the ellipsis of a variadic function.
+  SourceRange getParametersSourceRange() const;
----------------
nicolas wrote:
> aaron.ballman wrote:
> > Why does this omit the ellipsis? It's part of the parameter list and it seems like a strange design decision to leave that out of the source range for the parameter list.
> I haven't found a correct way to access the position of the ellipsis. There is no corresponding `ParmVarDecl` in `ParamInfo`.
> Did I miss something? It doesn't seem to be easily accessible.
I don't think you've missed anything; I think we need to track/dig out that information in order for this API to be usefully correct. I don't see a lot of value in getting a source range that's sometimes wrong.

I would recommend seeing if you can go back to the function type location information as-needed to figure out where the ellipsis is and include it in the range here.


================
Comment at: clang/unittests/AST/SourceLocationTest.cpp:676
+}
+
 TEST(CXXMethodDecl, CXXMethodDeclWithThrowSpecification) {
----------------
nicolas wrote:
> aaron.ballman wrote:
> > I'd like to see tests that include an ellipsis, as well as tests that use the preprocessor in creative ways. e.g.,
> > ```
> > #define FOO  int a, int b
> > 
> > void func(FOO);
> > void bar(float f, FOO, float g);
> > void baz(FOO, float f);
> > void quux(float f, FOO);
> > ```
> > Also, tests for member functions (both static and instance functions) and parameters with leading attributes would be useful. e.g.,
> > ```
> > void func([[]] int *i);
> > ```
> Source locations with macros always looked inconsistent to me. For example:
> ```
> #define RET int
> RET f(void);
> ```
> 
> Here, `getReturnTypeSourceRange` returns `<input.cc:2:1 <Spelling=input.cc:1:13>-input.cc:2:1 <Spelling=input.cc:1:13>>`, where I would expect (and I could be wrong) `<input.cc:2:1 <Spelling=input.cc:1:13>-input.cc:2:3 <Spelling=input.cc:1:16>>`
> 
> The same thing happens with `getParametersSourceRange`. Should I test the current behavior?
> 
> I added the other tests you requested.
I'd test current behavior.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63276/new/

https://reviews.llvm.org/D63276