njames93 abandoned this revision.
njames93 added a comment.
I have decided to move this behaviour into the ast matcher directly, I feel in
the long term, that should be a better home for this functionality.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.
aaron.ballman accepted this revision.
aaron.ballman added a comment.
LGTM
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:349-352
+ // This should be an assert, but asserts shouldn't be used in signal
+ // handlers
+ if (!CurContext)
+return;
---
njames93 added a subscriber: thakis.
njames93 added a comment.
@thakis I tried to test this on the gn build and got no issues, however I'm
aware that the CTCrashTestTrace plugin isn't even compiled on the gn build. Not
sure its worth the effort fixing that. The lit test should be disabled anyway
njames93 updated this revision to Diff 405636.
njames93 edited the summary of this revision.
njames93 added a comment.
Ignore the CTCrashTestTrace implementation file in the lit tests, Fixes 'Test
has no 'RUN'' error.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://re
njames93 added inline comments.
Comment at: clang-tools-extra/test/CMakeLists.txt:100
+
+ llvm_add_library(
+ CTCrashTestTrace
Fairly certain llvm_add_library isnt the correct function to use as that will
put the test module in the build/lib directory. How
njames93 updated this revision to Diff 405581.
njames93 added a comment.
Herald added a subscriber: mgorny.
Add testing infrastructure to verify stack trace output
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118520/new/
https://reviews.llvm.org/D
njames93 added inline comments.
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:239
+ const ast_matchers::BoundNodes &Result) const {
+if (CurrentlyProcessing)
+ CurrentlyProcessing->onProcessingCheckStart(CheckName, Re
njames93 updated this revision to Diff 405111.
njames93 added a comment.
Added release notes.
Remove AST dump of bound nodes, typically isn't very helpful
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118520/new/
https://reviews.llvm.org/D118520
F
LegalizeAdulthood accepted this revision.
LegalizeAdulthood added a comment.
This revision is now accepted and ready to land.
Still LGTM and my comments are just thought experiments,
not must haves before pushing.
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticCons
Eugene.Zelenko added a comment.
Sure, this is important improvement.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118520/new/
https://reviews.llvm.org/D118520
___
cfe-commits mailing list
cfe-commits@li
njames93 added a comment.
@Eugene.Zelenko Do you think I should update release notes for this?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118520/new/
https://reviews.llvm.org/D118520
___
cfe-commits m
njames93 updated this revision to Diff 404350.
njames93 marked 5 inline comments as done.
njames93 added a comment.
Refactored implementation to not store state in ClangTidyContext. This will let
external consumers of clang-tidy opt into the behaviour. As well as integrate
the reporting to any s
Eugene.Zelenko added inline comments.
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:191
+ }
+ void setContext(const ASTContext &Ctx) { CurContext = &Ctx; }
+ void clearContext() { CurContext = nullptr; }
Please separate with newline.
LegalizeAdulthood accepted this revision.
LegalizeAdulthood added a comment.
This revision is now accepted and ready to land.
LGTM with a few whitespace nits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118520/new/
https://reviews.llvm.org/D11852
LegalizeAdulthood added inline comments.
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:181
+ }
+ void onResultEntry(StringRef CheckName,
+ const ast_matchers::BoundNodes &BoundNodes) {
blank line before `onResultEn
LegalizeAdulthood added inline comments.
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:201
+} // namespace clang
ClangTidyError::ClangTidyError(StringRef CheckName,
ClangTidyError::Level DiagLevel,
Inser
njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, JonasToth, LegalizeAdulthood.
Herald added subscribers: carlosgalvezp, xazax.hun.
njames93 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Create a
17 matches
Mail list logo