ymandel marked 5 inline comments as done.
ymandel added inline comments.

================
Comment at: clang/include/clang/Tooling/FixIt.h:73
+// context. In contrast with \p getText(), this function selects a source range
+// "automatically", extracting text that a reader might intuitively associate
+// with a node.  Currently, only specialized for \p clang::Stmt, where it will
----------------
kimgr wrote:
> ymandel wrote:
> > ilya-biryukov wrote:
> > > What are other tricky cases you have in mind for the future?
> > I just assumed that we'd hit more as we dig into them, but, I'm not so sure 
> > now.  The two others I can think of offhand are 1) extending to include 
> > associated comments, 2) semicolons after declarations.  Commas present a 
> > similar challenge (if a bit simpler) when used in a list (vs. the comma 
> > operator).  Are there any other separators in C++? 
> > 
> > At a higher level, it would be nice to align this with your work on tree 
> > transformations. That is, think of these functions as short-term shims to 
> > simulate the behavior we'll ultimately get from that new library. However, 
> > it might be premature to consider those details here.
> Peanut gallery comment on this:
> 
> > The two others I can think of offhand are
> > 1) extending to include associated comments,
> > 2) semicolons after declarations.
> > Commas present a similar challenge (if a bit simpler) when used in a list 
> > (vs. the comma operator).
> > Are there any other separators in C++?
> 
> Would it make sense to let callers choose what level of expansion they want 
> with a flag mask? Somehow I think that makes it easier to name the function, 
> too, e.g.:
> ```
> StringRef getExpandedRange(const Stmt &S, ASTContext &Context, ExpansionFlags 
> Flags);
> ```
> 
Yes, I think that's a good idea. I even like the name except that it will 
likely be confused with Expansion locs.  Maybe `getExtendedRange`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58556



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to