student433 wrote: > General comments: > > * Please do not use tabs for spacing. This is what causes the unexpected > whitespace gaps in the diff. > > * There is a utility, clang-format-diff.py, which will assist in > appeasing the code formatting check. However, I noticed running it myself > that it changes some entire lists that should likely stay the way they are, > so be careful! > > * My usual command line is something like: git diff -U0 --no-color > --relative HEAD~1..HEAD | /path/to/clang-format-diff.py -p1 -i -binary > /path/to/clang-format(.exe) > > * Is there no desire to catch and support C2000 built-in functions? Those > will show up in clangd as undefined symbols and will be errors in c99 or > later, and in all C++ modes. > > * Options > > * The options parser for clang and for the TI compilers are obviously > very different. Some options interact with others for the purposes of > ease-of-use and error checking, which is something that the clang options > parser won't understand. I'll do my best to be diligent in case I have to > point this out > * There are many more options than the ones currently captured. Is it > important we get as many as we can? Otherwise this will be a PR that is > tailor-fit to a single use-case. > > * Is there a testing strategy you have in mind? At the very least, we > need to cover the selection of the new mode and options. See > clang/test/Driver/cl-options.c for an example
Thank you very much for your inputs, I have yet to add the test cases for cl2000 driver in clang/tests/Driver and pair it with diagnostics as needed. I will be sure to check the clang-format for indentation and other things. Will respond to each comment individually :) > Excited to see your nice work!!! > > AFAIK all _TI extension keywords_ may confuse clang frontend, and treating > them as macros would result in strange behaviors, e.g. > > ```c > // DSP2833x_PieVect.h > typedef interrupt void(*PINT)(void); > ``` > > So special keywords handling would be necessary... > > See [SPRU514Z - TMS320C28x Optimizing C/C++ > Compiler](https://www.ti.com/lit/ug/spru514z/spru514z.pdf): > > > 6.5 Keywords > > The C28x C/C++ compiler supports all of the standard C89 keywords, > > including const, volatile, and register. It supports all of the standard > > C99 keywords, including inline and restrict. It supports all of the > > standard C11 keywords. It also supports TI extension keywords > > `__interrupt`, `__cregister`, and `__asm`. > > ... > > 6.5.2 The `__cregister` Keyword > > ... > > 6.5.3 The `__interrupt` Keyword > > ... > > CMIIW :) @DragonDisciple Thank you very much for your inputs, I understand that these are semantically incorrect and would require a meaningful definition to avoid it being misunderstood. I could look into this :) https://github.com/llvm/llvm-project/pull/125663 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
