ymandel abandoned this revision.
ymandel marked an inline comment as done.
ymandel added inline comments.
================
Comment at: clang/include/clang/Tooling/Refactoring/NodeId.h:29
+public:
+ explicit NodeId(std::string Id) : Id(std::move(Id)) {}
+
----------------
ilya-biryukov wrote:
> What are the use-cases for passing a custom id to this class?
> If the purpose is to hide the IDs from the user, exposing this constructor
> seems to be against this goal.
If there are errors relating to the id (say, referencing it in a stencil when
it wasn't bound in the match), the errors are hard to understand when the id is
auto-generated. In general, I'd say that best practice is to explicitly specify
the id so that dynamic failures print a meaningful id. The default constructor
is a compromise and perhaps shouldn't be exported.
that said, if we could find another way to give good error messages (for
example, if we could somehow grab the line number of the declaration), then the
default construction would be the preferred approach.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59329/new/
https://reviews.llvm.org/D59329
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
