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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 26 06:52:49 PDT 2019


aaron.ballman added inline comments.


================
Comment at: clang/include/clang/AST/Decl.h:1928
+  /// macro. Otherwise, it returns the location of the end of the ellipsis.
+  SourceRange getEllipsisSourceRange() const {
+    const auto *FPT = getType()->getAs<FunctionProtoType>();
----------------
Why a source range? The ellipsis is a single token, so a range should be unnecessary. I would just have this return `FPT->getEllipsisLoc()`.


================
Comment at: clang/include/clang/AST/Decl.h:2353
+  /// Attempt to compute an informative source range covering the
+  /// function parameters. The source range is invalid if there are no
+  /// parameters.
----------------
Can you add "excluding the parentheses" to this as well, to make it crystal clear what is covered?


================
Comment at: clang/lib/AST/Decl.cpp:3307
+  unsigned NP = getNumParams();
+  auto EllipsisRange = getEllipsisSourceRange();
+
----------------
Don't use `auto` unless the type is explicitly spelled out in the initialization (elsewhere as well).


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

https://reviews.llvm.org/D63276





More information about the cfe-commits mailing list