[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

ffrankies added a subscriber: alexandre.isoard. ffrankies added a comment. Herald added a subscriber: usaxena95. @alexandre.isoard wrote: > I'm not sure what is the advantage of this compared to -Wpadded? This option only warns when padding exists. Our check does two things; it warns when

Re: [PATCH] D66564: [clang-tidy] new FPGA struct pack align check

I'm not sure what is the advantage of this compared to -Wpadded? On Sun, Sep 15, 2019 at 12:05 PM Roman Lebedev via Phabricator via llvm-commits wrote: > lebedev.ri added a comment. > > Forgot the most important question. > Right now this will fire on every single struct. > But it won't matter

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

lebedev.ri added a comment. Forgot the most important question. Right now this will fire on every single struct. But it won't matter unless the alignment/size actually matters, and most often that will happen when you have e.g. a vector of such structs. What i'm asking is - should this be more

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

lebedev.ri added a comment. I, too, don't believe this is FPGA specific; it should likely go into `misc-` or even `performance-`. The wording of the diags seems weird to me, it would be good to 1. add more explanation to the docs and 2. reword the diags. Comment at:

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

ffrankies marked 22 inline comments as done. ffrankies added a comment. In D66564#1647779 , @BlackAngel35 wrote: > > Please mention new module and check in Release Notes. The new module and check are now mentioned in Release Notes. P.S. The above

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

ffrankies updated this revision to Diff 219358. ffrankies added a comment. Herald added subscribers: kadircet, jkorous. Sorry for the delay. I was mostly having trouble building clang-tidy after pulling from master and having it recognize that the struct-pack-align check exists. I finally

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

riccibruno added a comment. I do not understand why this check is specific to FPGAs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

BlackAngel35 requested changes to this revision. BlackAngel35 added a comment. This revision now requires changes to proceed. In D66564#1640584 , @Eugene.Zelenko wrote: > Please mention new module and check in Release Notes. CHANGES SINCE LAST

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

alexfh added a comment. Thanks for the contribution and welcome to the LLVM community! A few more comments from me. I hope, this will help to tune to the LLVM coding style and conventions. This doc may be useful to read, if you haven't done so already: http://llvm.org/docs/CodingStandards.html

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:72 + + Contains lint checks related to OpenCL programming for FPGAs. + Just checks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

ffrankies updated this revision to Diff 217224. ffrankies added a comment. Added space after clang-tidy in header comments, updated check documentation to use link syntax. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564 Files:

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

Eugene.Zelenko added inline comments. Comment at: clang-tidy/fpga/StructPackAlignCheck.h:1 +//===--- StructPackAlignCheck.h - clang-tidy-*- C++ -*-===// +// Please add space after clang-tidy. Same in source file. Comment

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

ffrankies updated this revision to Diff 216928. ffrankies added a comment. Implemented changes requested by Eugene.Zelenko CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564 Files: clang-tidy/fpga/CMakeLists.txt

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

Eugene.Zelenko added inline comments. Comment at: clang-tidy/fpga/FPGATidyModule.cpp:15 + + +using namespace clang::ast_matchers; Unnecessary empty line. Comment at: clang-tidy/fpga/FPGATidyModule.cpp:32 + +} // namespace flocl +

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

Eugene.Zelenko added a comment. Please mention new module and check in Release Notes. Comment at: clang-tidy/fpga/FPGATidyModule.cpp:5 +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details.