Hi,
1.) Does not depend on the existence of and/or adherence to any customs/practices/coding styles, which can and do change over time, or are simply overlooked, causing errors that people will have to correct manually or even cause silent failure that no-one notices.
2.) Is fully dynamic, and does not have any paths 'hard-coded' into the script, which one would have to adjust if in the future the paths/names change. I already am worried about the fact that the approach uses hard-coded defines (-DFOO/-UBAR) on the commandline instead of determining them dynamically, but I currently see no way to do this differently, so I guess that has to stay for now. Also, I guess the chance that these defines change over time is far less likely than the chance that directory names and locations change.
But then I decided to put my money where my mouth is, and made the necessary changes to the cppcheck-report script, and guess what ? Apart from approach differences, your version completed in an acceptable execution time, whereas my approach took *ages* (after 40 minutes cppcheck was still only about 1% completed, after which I aborted the attempt).
As of now, I cannot tell what causes this massive difference in execution time, but the only visible difference in cppcheck output was this :
my version: Checking basctl/source/basicide/basides1.cxx: LINUX=1;__GNUC__=1;UNX=1;NDEBUG=1;CORE_LITTLE_ENDIAN=1;__LITTLE_ENDIAN__=1;__x86_64__=1;LINUX=1;__GNUC__=1;UNX=1;NDEBUG=1;CORE_LITTLE_ENDIAN=1;__LITTLE_ENDIAN__=1;__x86_64__=1...
your version: Checking basctl/source/basicide/basides1.cxx ...
Obvious things: 'my version' listed the -D in the output, your's did not. 'My version' lists the defines *twice* (which can't be good).
- Maarten
On Sun, Sep 30, 2018 at 5:20 AM Luke Benes <lukebenes@xxxxxxxxxxx> wrote:
Maarten,
Thanks for your suggestion here and your earlier contributions to the Cppcheck Report. I agree that we should create the include file dynamically. However the approach used by your script seems like overkill. Cppcheck already finds that quoted includes like
#include "GraphicExportFilter.hxx"
.
Also when there seems to have been a coding style that all <> includes outside of /inc folders should be defined by their relative path. Cppcheck only complains about 4 missing includes that do not follow this pattern.(see my earlier email on oddball includes).
Unless, I'm missing something, I still prefer this approach:
$ find . -type d \( -name "inc" -o -name "include" \) |sort > inc.txt
inc.txt only has ~200 entries, where as /tmp/tmpfile.txt has ~1,800 after sorting it.
-Luke
I was about to speak of the supposedly preferred approach that I took, for the reasons that my approach :
1.) Does not depend on the existence of and/or adherence to any customs/practices/coding styles, which can and do change over time, or are simply overlooked, causing errors that people will have to correct manually or even cause silent failure that no-one notices.
2.) Is fully dynamic, and does not have any paths 'hard-coded' into the script, which one would have to adjust if in the future the paths/names change. I already am worried about the fact that the approach uses hard-coded defines (-DFOO/-UBAR) on the commandline instead of determining them dynamically, but I currently see no way to do this differently, so I guess that has to stay for now. Also, I guess the chance that these defines change over time is far less likely than the chance that directory names and locations change.
But then I decided to put my money where my mouth is, and made the necessary changes to the cppcheck-report script, and guess what ? Apart from approach differences, your version completed in an acceptable execution time, whereas my approach took *ages* (after 40 minutes cppcheck was still only about 1% completed, after which I aborted the attempt).
As of now, I cannot tell what causes this massive difference in execution time, but the only visible difference in cppcheck output was this :
my version: Checking basctl/source/basicide/basides1.cxx: LINUX=1;__GNUC__=1;UNX=1;NDEBUG=1;CORE_LITTLE_ENDIAN=1;__LITTLE_ENDIAN__=1;__x86_64__=1;LINUX=1;__GNUC__=1;UNX=1;NDEBUG=1;CORE_LITTLE_ENDIAN=1;__LITTLE_ENDIAN__=1;__x86_64__=1...
your version: Checking basctl/source/basicide/basides1.cxx ...
Obvious things: 'my version' listed the -D in the output, your's did not. 'My version' lists the defines *twice* (which can't be good).
I don't know what's going on here.
- Maarten
_______________________________________________ LibreOffice mailing list LibreOffice@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/libreoffice
