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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: aaron.ballman.
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;
----------------
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.


================
Comment at: clang/lib/AST/Decl.cpp:3305
+SourceRange FunctionDecl::getParametersSourceRange() const {
+  auto NP = getNumParams();
+  if (NP == 0)
----------------
Please don't use `auto` here (or below); the type is not spelled out in the initialization.


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63276