[PATCH] D52524: Add -Wno-poison-system-directories flag

denik updated this revision to Diff 215260. denik added a comment. Herald added a subscriber: ormris. Updated the code (removed Diag propagation). Added test cases. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 Files:

[PATCH] D52524: Add -Wno-poison-system-directories flag

denik updated this revision to Diff 215271. denik added a comment. Fixed clang-format. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 Files: clang/include/clang/Basic/DiagnosticCommonKinds.td clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D52524: Add -Wno-poison-system-directories flag

denik added a comment. Manoj, please check updated diff. Comment at: clang/test/Frontend/warning-poison-system-directories.c:12 +// Missing target but included sysroot still causes the warning. +// RUN: %clang -Wpoison-system-directories -I/usr/include --sysroot

[PATCH] D52524: Add -Wno-poison-system-directories flag

denik updated this revision to Diff 215958. denik marked 3 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 Files: clang/include/clang/Basic/DiagnosticCommonKinds.td clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D52524: Add -Wno-poison-system-directories flag

denik updated this revision to Diff 215945. denik added a comment. Changed Wpoison-system-directories warning to be disabled by default. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 Files: clang/include/clang/Basic/DiagnosticCommonKinds.td

[PATCH] D52524: Add -Wno-poison-system-directories flag

denik marked 2 inline comments as done. denik added inline comments. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1071 +// cross-compiling. +def PoisonSystemDirectories : DiagGroup<"poison-system-directories">; + manojgupta wrote: > Please verify

[PATCH] D52524: Add -Wpoison-system-directories warning

denik updated this revision to Diff 216021. denik added a comment. Removed check for libraries. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 Files: clang/include/clang/Basic/DiagnosticCommonKinds.td

[PATCH] D52524: Add -Wpoison-system-directories warning

denik updated this revision to Diff 217746. denik marked 4 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 Files: clang/include/clang/Basic/DiagnosticCommonKinds.td clang/lib/Frontend/InitHeaderSearch.cpp

[PATCH] D52524: Add -Wpoison-system-directories warning

denik added a comment. Hi Aaron, Thank you for your review. I updated the diff with suggested changes. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1072 +// cross-compiling. +def PoisonSystemDirectories : DiagGroup<"poison-system-directories">; +

[PATCH] D52524: Add -Wpoison-system-directories warning

denik added a comment. Ping @aaron.ballman , please verify the change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52524: Add -Wpoison-system-directories warning

denik updated this revision to Diff 219924. denik added a comment. Combined two if into one. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 Files: clang/include/clang/Basic/DiagnosticCommonKinds.td clang/lib/Frontend/InitHeaderSearch.cpp

[PATCH] D52524: Add -Wpoison-system-directories warning

denik marked 2 inline comments as done. denik added inline comments. Comment at: clang/lib/Frontend/InitHeaderSearch.cpp:141-143 + if (HasSysroot) { +if (MappedPathStr.startswith("/usr/include") || +MappedPathStr.startswith("/usr/local/include")) {

[PATCH] D52524: Add -Wno-poison-system-directories flag

denik commandeered this revision. denik added a reviewer: yunlian. denik added a comment. Herald added a project: clang. Taking ownership of the change as Yunlian is no longer working on this CL. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

denik added a comment. We are hitting the warning in template code with bool and loop like this: "for (const auto& element : value)" where element is bool from template. But not always. When it is bool the warning triggers: error: loop variable 'element' is always a copy because the range of

[PATCH] D158006: [Clang][WIP]Experimental implementation of data member packs in dependent context.

denik added inline comments. Comment at: clang/lib/Sema/SemaExprMember.cpp:528-529 +if (Field->getDeclName() == NameInfo.getName()) { + if (const PackExpansionType *PET = + dyn_cast(Field->getType())) { +isMemberPack = true;

[PATCH] D128372: [Clang-Tidy] Empty Check

denik accepted this revision. denik added a comment. Thanks Abraham! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___ cfe-commits mailing list

[PATCH] D128372: [Clang-Tidy] Empty Check

denik added a comment. Thanks for adding a check! Please check my comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:76 + +auto Methods = Arg->getType()->getAsCXXRecordDecl()->methods(); +auto Clear = llvm::find_if(Methods, [](const

[PATCH] D129886: [clang] Add -fdiagnostics-format=sarif option for future SARIF output

denik added a comment. Thanks for adding the flag! Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1383 + +def SarifFormatUnstable : DiagGroup<"sarif-format-unstable">; cjdb wrote: > Please make sure that there's a newline at the end of each file. I

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

denik added inline comments. Comment at: clang/include/clang/Frontend/SARIFDiagnostic.h:24-26 + raw_ostream *OS; + + SarifDocumentWriter *Writer; cjdb wrote: > These can go in the private section below. In addition, I think it's worth putting a comment here

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

denik added inline comments. Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:155 +break; + } +} vaibhav.y wrote: > Does this need an `llvm_unreachable` after the switch? I guess no, because of `break`s. But... Although `case DiagnosticsEngine::Ignored`

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

denik added a comment. Thanks Abraham! Please mention in the summary that some part of SARIF Diag implementation was copied from Text Diag and further refactoring is needed. Also, like we discussed: - drop the unittest; - replace strict match of json output in FileCheck with partial matches

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

denik added inline comments. Comment at: clang/lib/Frontend/SARIFDiagnosticPrinter.cpp:71 + // other infrastructure necessary when emitting more rich diagnostics. + if (!Info.getLocation().isValid()) { // TODO: What is this case? +//

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

denik accepted this revision. denik added inline comments. Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:58 + + if (Loc.isValid()) +Result = addLocationToResult(Result, Loc, PLoc, Ranges, *Diag); abrahamcd wrote: > denik wrote: > > I think we should

[PATCH] D128372: [Clang-Tidy] Empty Check

denik added a comment. @Eugene.Zelenko , could you please stamp the patch if you don't have any other concerns? @njames93 , if you don't have spare cycles could you please suggest other reviewers? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D128372: [Clang-Tidy] Empty Check

denik added a comment. Any updates from the reviewers? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D128372: [Clang-Tidy] Empty Check

denik added a comment. @LegalizeAdulthood, @njames93, is there anything else we should address in this change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372

[PATCH] D128372: [Clang-Tidy] Empty Check

denik added a comment. @LegalizeAdulthood, @njames93, friendly ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___ cfe-commits mailing list

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

denik added a comment. In D131084#3697256 , @cjdb wrote: > In D131084#3697211 , @vaibhav.y > wrote: > >> Submitting for review: >> >> Some notes: >> >> There are a couple of ways I think we can acheive this, per

[PATCH] D141107: [clang-tidy] don't warn when returning the result for bugprone-standalone-empty

denik accepted this revision. denik added a comment. Thanks @v1nh1shungry for the fix! The change and test cases look good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141107/new/ https://reviews.llvm.org/D141107

[PATCH] D128372: [Clang-Tidy] Empty Check

denik updated this revision to Diff 479684. denik added a comment. Replaced deprecated [[@LINE-1]] Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 Files:

[PATCH] D128372: [Clang-Tidy] Empty Check

denik updated this revision to Diff 479677. denik marked 2 inline comments as done. denik added a comment. Format changes. Removed blank lines and fixed test types. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/

[PATCH] D128372: [Clang-Tidy] Empty Check

denik added a comment. In D128372#3965162 , @MaskRay wrote: > I just took a glance and the code looks reasonable. @MaskRay thanks for the review! I have updated the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142423: [clang-tidy] Fix segfault in bugprone-standalone-empty

denik created this revision. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. denik requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. The check incorrectly

[PATCH] D146654: [clang] replaces numeric SARIF ids with heirarchical names

denik added inline comments. Comment at: clang/lib/Basic/DiagnosticIDs.cpp:103 +// Stable names +const char *const StaticDiagInfoStableNames[] = { +#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \ As of now it's 4600 strings,

[PATCH] D151162: Add -Wpacked-non-pod to -Wall

denik added inline comments. Comment at: clang/test/CodeGenCXX/warn-padded-packed.cpp:4 +// -Wpacked-non-pod itself should not emit the "packed attribute is unnecessary" warnings. +// RUN: %clang_cc1 -triple=x86_64-none-none -Wpacked-non-pod -verify=top %s -emit-llvm-only +//

[PATCH] D151162: Add -Wpacked-non-pod to -Wall

denik accepted this revision. denik added a comment. Thanks @SlaterLatiao! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151162/new/ https://reviews.llvm.org/D151162 ___ cfe-commits mailing list

[PATCH] D151575: [clang][diagnostics] Always show include stacks on errors

denik accepted this revision. denik added a comment. This revision is now accepted and ready to land. As long as having `In file included from` on each error **without notes** from one include file is fine, I don't see any problem with this. LGTM. But let's check what others think about it. For