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
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
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
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:
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
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:
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
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
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
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
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
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
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
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
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
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
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:
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
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
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
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:
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
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
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
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
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:
26 matches
Mail list logo