[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Jonathan B Coe via Phabricator via cfe-commits
jbcoe marked an inline comment as done. jbcoe added inline comments. Comment at: clang-tools-extra/clang-tidy/safety/NoAssemblerCheck.cpp:13 +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + Eugene.Zelenko wrote: > aaron.ballman

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Jonathan B Coe via Phabricator via cfe-commits
jbcoe added a comment. @aaron.ballman I will address post-approval comments post-commit. Repository: rL LLVM https://reviews.llvm.org/D29267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. There were review comments still outstanding when you commit the patch. Can you please address those post-commit? Repository: rL LLVM https://reviews.llvm.org/D29267 ___ cfe-commits mailing list

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Jonathan B Coe via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL294255: [clang-tidy] safety-no-assembler (authored by jbcoe). Changed prior to commit: https://reviews.llvm.org/D29267?vs=87207=87314#toc Repository: rL LLVM https://reviews.llvm.org/D29267 Files:

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/safety/NoAssemblerCheck.cpp:13 +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + aaron.ballman wrote: > Is this include needed? Will be good idea to run

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/safety/NoAssemblerCheck.cpp:13 +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + Is this include needed? Comment at:

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Paweł Żukowski via Phabricator via cfe-commits
idlecode accepted this revision. idlecode added a comment. This revision is now accepted and ready to land. Sure, but I don't have commit rights so someone else have to push it :) https://reviews.llvm.org/D29267 ___ cfe-commits mailing list

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Jonathan B Coe via Phabricator via cfe-commits
jbcoe added a comment. @idlecode please can you approve if it LGTY? https://reviews.llvm.org/D29267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In https://reviews.llvm.org/D29267#667487, @alexfh wrote: > I wonder whether there's a compiler diagnostic for this purpose. Compiler > diagnostics are more efficient at reaching users and should be preferred > where they are appropriate (this seems like one of

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Paweł Żukowski via Phabricator via cfe-commits
idlecode added a comment. LGTM. HIC++ Standard seems worth implementing. https://reviews.llvm.org/D29267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. In https://reviews.llvm.org/D29267#667614, @jbcoe wrote: > @alexfh Can we defer moving this to a compiler diagnostic? I'm keen to get a > target in place for people to write more safety checks. +1 from me :) and assembler might be bad style, but is it worth a

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Jonathan B Coe via Phabricator via cfe-commits
jbcoe added a comment. @alexfh Can we defer moving this to a compiler diagnostic? I'm keen to get a target in place for people to write more safety checks. https://reviews.llvm.org/D29267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. alright :) https://reviews.llvm.org/D29267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Jonathan B Coe via Phabricator via cfe-commits
jbcoe added a comment. @JonasToth My main intention with this patch is to provide such a starting point. A few people have mentioned that they'd be keen to contribute checks. https://reviews.llvm.org/D29267 ___ cfe-commits mailing list

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. out of curiousity: i would like to contribute to the safety module as well. but currently there is no starting point in master. is there a way to get some code that i can start with? I think the module file where the registration happens would be enough. this is not

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Jonathan B Coe via Phabricator via cfe-commits
jbcoe marked an inline comment as done. jbcoe added inline comments. Comment at: clang-tools-extra/clang-tidy/safety/NoAssemblerCheck.cpp:32 + + diag(ASM->getAsmLoc(), "'%0' is an inline assembler statement") << SourceText; +} aaron.ballman wrote: > The

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Jonathan B Coe via Phabricator via cfe-commits
jbcoe updated this revision to Diff 87207. jbcoe marked an inline comment as done. jbcoe added a comment. Improve diagnostic message. Find other sorts of inline assembler. Minor fixes for other review comments. https://reviews.llvm.org/D29267 Files:

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-06 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. I wonder whether there's a compiler diagnostic for this purpose. Compiler diagnostics are more efficient at reaching users and should be preferred where they are appropriate (this seems like one of such cases). https://reviews.llvm.org/D29267

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/safety/NoAssemblerCheck.cpp:32 + + diag(ASM->getAsmLoc(), "'%0' is an inline assembler statement") << SourceText; +} The diagnostic text doesn't help the user to understand why the

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-04 Thread Jonathan B Coe via Phabricator via cfe-commits
jbcoe planned changes to this revision. jbcoe added inline comments. Comment at: clang-tools-extra/test/clang-tidy/safety-no-assembler.cpp:2 +// RUN: %check_clang_tidy %s safety-no-assembler %t + +void f() { idlecode wrote: > Maybe this check should handle

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-04 Thread Paweł Żukowski via Phabricator via cfe-commits
idlecode added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:156 + safety-no-assembler + safety-no-vector-bool `safety-no-vector-bool` seems to belong to the other patch. Comment at:

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-03 Thread Jonathan B Coe via Phabricator via cfe-commits
jbcoe added a comment. High Integrity C++ is often used as a standard for safety-critical systems. High Integrity C++ requires no assembler due to portability issues. Not my choice of wording. I plan to implement more of the High Integrity C++ checks, some of which are more obviously

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-02-03 Thread Paweł Żukowski via Phabricator via cfe-commits
idlecode added a comment. Standard you linked mentions portability as the reason inline assembler should be avoided. Should it really be named 'safety'? https://reviews.llvm.org/D29267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-01-30 Thread Jonathan B Coe via Phabricator via cfe-commits
jbcoe updated this revision to Diff 86289. jbcoe added a comment. Add link to HIC++ website and fix release notes. https://reviews.llvm.org/D29267 Files: clang-tools-extra/clang-tidy/CMakeLists.txt clang-tools-extra/clang-tidy/safety/CMakeLists.txt

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-01-29 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:60 -The improvements are... +Added a safety module for safety-critical checks like those from High Integrity C++. + Please take look on Release Notes format in previous

[PATCH] D29267: [clang-tidy] safety-no-assembler

2017-01-29 Thread Jonathan B Coe via Phabricator via cfe-commits
jbcoe created this revision. jbcoe added a project: clang-tools-extra. Herald added subscribers: JDevlieghere, mgorny. Add a new clang-tidy module for safety-critical checks. Include a check for inline assembler. https://reviews.llvm.org/D29267 Files: