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

Wed Apr 7 03:44:17 PDT 2021

benshi001 marked 2 inline comments as done.
benshi001 added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/AVR.cpp:356

+void AVRToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
+                                             ArgStringList &CC1Args) const {
----------------
Anastasia wrote:
> benshi001 wrote:
> > Anastasia wrote:
> > > benshi001 wrote:
> > > > Anastasia wrote:
> > > > > function name doesn't adhere to the coding style.
> > > > Not sure your concern ? Other platforms also use the same function name.
> > > > 
> > > > ```
> > > > void RISCVToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
> > > >                                                ArgStringList &CC1Args) const {
> > > > void Linux::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
> > > >                                       ArgStringList &CC1Args) const {
> > > > void WebAssembly::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
> > > >                                             ArgStringList &CC1Args) const {
> > > > ```
> > > https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
> > > 
> > > 
> > > 
> > > > Function names should be verb phrases (as they represent actions), and command-like function should be imperative. The name should be camel case, and start with a lower case letter (e.g. openFile() or isFoo()).
> > > 
> > > 
> > > You should adhere to the coding style in the new functionality.
> > > 
> > I have created another patch for that, and would like to keep current patch simple without noise.
> > 
> > https://reviews.llvm.org/D100022
> Sorry, my mistake - I haven't realized that you are overriding the method instead of adding a new one. Then I think it is best to keep the old style because we don't normally do purely formatting/renaming changes.
Thanks. You are appreciated to go on with this patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97669