[cfe-dev] Problem parsing a simple C++ file.

[Michael Spencer]

On Tue, Dec 4, 2012 at 2:32 AM, Manuel Klimek <klimek at google.com> wrote:
> On Mon, Dec 3, 2012 at 7:46 PM, Sean Silva <silvas at purdue.edu> wrote:
>>
>> Ah, I see, the initial review precedes my gmail log.
>>
>> The natural thing for me seems like it would just be to add a
>> changeDirectory() function to libSystem. Was this not considered?

This was considered, but chdir is evil due to its hatred of threads.
The alternative needs directory handles, which we have yet to do, but
Chandler and I were just discussing this in #llvm last night.

I'm fine with the current code as a short-term solution. I'm also fine
with making it work on Windows by using ChangeDirectory.

- Michael Spencer

>
>
> +Michael
> There was  definitely some talk about mid- / long-term strategies, but I
> think we didn't find a good short-term solution beside adding the non-nice
> system includes.
>
> One thing we'll need to do anyway is make clang thread-safe regarding chdir,
> which means that we'll need an interface that supports keeping state between
> calls so that changes to the directory (like symlinks changing during a
> compile) do not affect the compilation.
>
> Cheers,
> /Manuel
>
>>
>> -- Sean Silva
>>
>> On Mon, Dec 3, 2012 at 5:11 AM, Manuel Klimek <klimek at google.com> wrote:
>> > On Mon, Dec 3, 2012 at 8:23 AM, Sean Silva <silvas at purdue.edu> wrote:
>> >>
>> >> >
>> >> >
>> >> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?view=markup
>> >>
>> >> Ugh. Why is this directly including system headers? That's a blatant
>> >> violation of
>> >> <http://llvm.org/docs/SystemLibrary.html#don-t-include-system-headers>.
>> >>
>> >> Manuel, what kind of review did this code get before being checked in?
>> >> I can't seem to find any review thread leading up to r154008.
>> >
>> >
>> >
>> > http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20120123/051828.html
>> > (note that this review went over 4 months, so the mail archive can't
>> > follow
>> > it through; use site:lists.cs.uiuc.edu "[PATCH] Implements support to
>> > run
>> > standalone tools" in your favorite search engine to find all parts of
>> > the
>> > review)
>> >
>> > Note also that the lines you object to are not in the original patch.
>> > The
>> > patch that made those includes necessary was reviewed here:
>> >
>> > http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20120507/057330.html
>> > (again, the mail archive doesn't tell the full story)
>> >
>> > If you have a better solution, feel free to make suggestions.
>> >
>> > Cheers,
>> > /Manuel
>
>