[cfe-commits] r160265 - in /cfe/trunk: include/clang/Tooling/CommandLineClangTool.h lib/Tooling/CMakeLists.txt lib/Tooling/CommandLineClangTool.cpp test/CMakeLists.txt test/Tooling/clang-ast-dump.cpp tools/CMakeLists.txt tools/Makefile tools/clan

[Manuel Klimek]

On Thu, Jul 26, 2012 at 5:23 PM, Douglas Gregor <dgregor at apple.com> wrote:
>
> On Jul 26, 2012, at 12:52 AM, Manuel Klimek <klimek at google.com> wrote:
>
>> On Thu, Jul 26, 2012 at 5:06 AM, Alexander Kornienko <alexfh at google.com> wrote:
>>> On Wed, Jul 25, 2012 at 11:04 PM, Manuel Klimek <klimek at google.com> wrote:
>>>> High-level design of the new feature:
>>>> Are we sure that "substring of the node" is the desired interface? Why
>>>> not regexp? (heck, I'd love a dynamic matcher expression :P).
>>>
>>> Substring is quite useful, obvious and easy to implement. Regexps are
>>> better, but this could be in one of the next iterations. Dynamic matcher
>>> expressions would be great except that this would greatly increase skills
>>> threshold required to use this tool ;)
>>
>> Yea, I was not serious about the matcher expressions. And, in the case
>> of an extra tool, I'd have been fine with this. For Clang itself, I'm
>> less sure that we're as happy to go and change stuff around.
>>
>> Doug? Any comment on how happy we are with having a command line
>> interface we know we'll want to change later?
>
> I think it's fine to revise the command-line interface over time. This is a debugging/exploration option after all, not something that users should depend on as part of their tools.

Ok, sounds good. With that I don't have any nits on the change any
more - ready to go in?

Thanks,
/Manuel

>>>> -ASTConsumer *CreateASTPrinter(raw_ostream *OS);
>>>> +ASTConsumer *CreateASTPrinter(raw_ostream *OS, const std::string
>>>> &FilterString);
>>>>
>>>> Any reasons not to use StringRef?
>>>
>>> The reason is that performance issues are not going to appear in this code,
>>> which is invoked once per program execution.
>>
>> My impression is that Clang has a "use StringRef unless you have a
>> good reason not to". With that in place, not using StringRef makes
>> readers of the code stop and think why you did it.
>
> Yes, we prefer StringRef wherever it can be used.
>
>         - Doug
>