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

[Nicolas Manichon via Phabricator via cfe-commits]

nicolas marked an inline comment as done.
nicolas 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;
----------------
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.


================
Comment at: clang/unittests/AST/SourceLocationTest.cpp:676
+}
+
 TEST(CXXMethodDecl, CXXMethodDeclWithThrowSpecification) {
----------------
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.


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

https://reviews.llvm.org/D63276