kimgr 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 ---------------- ymandel wrote: > 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`? Extended sounds good to me too! 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
