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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
