[PATCH] D97669: [clang][AVR] Add avr-libc/include to clang system include paths

[Ben Shi via Phabricator via cfe-commits]

benshi001 added a comment.

In D97669#2688771 <https://reviews.llvm.org/D97669#2688771>, @Anastasia wrote:

> In D97669#2687419 <https://reviews.llvm.org/D97669#2687419>, @benshi001 wrote:
>
>> In D97669#2685698 <https://reviews.llvm.org/D97669#2685698>, @Anastasia wrote:
>>
>>> In D97669#2678460 <https://reviews.llvm.org/D97669#2678460>, @benshi001 wrote:
>>>
>>>> In D97669#2676826 <https://reviews.llvm.org/D97669#2676826>, @Anastasia wrote:
>>>>
>>>>> In D97669#2665865 <https://reviews.llvm.org/D97669#2665865>, @benshi001 wrote:
>>>>>
>>>>>> In D97669#2661560 <https://reviews.llvm.org/D97669#2661560>, @Anastasia wrote:
>>>>>>
>>>>>>> Is `stdio.h`  used by anything?
>>>>>>
>>>>>> No. `stdio.h` is not used. But if I do `#include <avr/interrupt.h>`, then `-I /usr/lib/avr/include` must be specified in the command line option. While avr-gcc does not reqiures that.
>>>>>>
>>>>>> I would like to keep clang & avr-gcc in the behaviour.
>>>>>
>>>>> Ok, do you plan to use it later? Then perhaps you should be adding it in the subsequent patches?
>>>>
>>>> No. No future pathes are needed. The avr-libc includes standard libc (headers and libs) and avr specific headers and libs. This patch fixes all of these issues. Both standard libc and avr platform specific libs can be used without any explict command line option, just like avr-gcc does.
>>>
>>> Ok, so your patch is adding the following file
>>>
>>> `clang/test/Driver/Inputs/basic_avr_tree/usr/lib/avr/include/stdio.h`
>>>
>>> But it doesn't seem to be used at present? If you don't need it anywhere it should not be added.
>>
>> No. The `stdio.h` is still needed even it is not referred by any code in current patch. It works as a placeholder for the directory `clang/test/Driver/Inputs/basic_avr_tree/usr/lib/avr/include/`, and this directory is needed in the test case `clang/test/Driver/avr-toolchain.c` of current patch, and this directory can not exist without `stdio.h`.
>
> Do you know why an empty directory is not sufficient?
>
> Overall perhaps it makes more sense if someone with more experience on AVR reviews this change.

I just learnt that llvm repo uses an empty .keep file to keep an empty directory, so I replaced the stdio.h with a .keep. Thanks for your comments.


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

https://reviews.llvm.org/D97669