[PATCH] D111570: [clang/CFG] Don't explicitly add AttributedStmtClass to AlwaysAddList

thakis created this revision. thakis added a reviewer: hans. thakis requested review of this revision. CFGBuilder::addStmt() implicitly passes AddStmtChoice::AlwaysAdd to Visit() already, so this should have no behavior change. https://reviews.llvm.org/D111570 Files: clang/lib/Sema/AnalysisBa

[PATCH] D111568: [clang] Omit most AttributedStatements from the CFG

thakis added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:2410 + +CFGBlock *CFGBuilder::VisitAttributedStmt(AttributedStmt *A, + AddStmtChoice asc) { (Compare this function to `CFGBuilder::VisitStmt` in this fil

[PATCH] D111545: [Clang][NFC] Fix multiline comment prefixes in function headers

thakis added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:118 AddStaticDeviceLibsLinking(C, *this, JA, Inputs, Args, CmdArgs, "amdgcn", - SubArchName, - /* bitcode SDL?*/ true, - /* Pos

[PATCH] D111568: [clang] Omit most AttributedStatements from the CFG

thakis added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:2418-2420 + // also no children, and omit the others. None of the other current StmtAttrs + // have semantic meaning for the CFG. + if (isFallthroughStatement(A) && asc.alwaysAdd(*this, A)) {

[PATCH] D111570: [clang/CFG] Don't explicitly add AttributedStmtClass to AlwaysAddList

thakis added a comment. Yes, that's my understanding. (I'm not super familiar with this code either.) The line was added in the change that added -Wimplicit-fallthrough (84837d5b5aa0d), and all the -Wimplicit-fallthrough tests still pass with this. Chances are it was unnecessary back then too.

[PATCH] D111568: [clang] Omit most AttributedStatements from the CFG

thakis marked an inline comment as done. thakis added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:2407 + return hasSpecificAttr(A->getAttrs()) && + isa(A->getSubStmt()); +} hans wrote: > Can fallthrough statements ever have children? If not,

[PATCH] D111568: [clang] Omit most AttributedStatements from the CFG

thakis updated this revision to Diff 378996. thakis added a comment. rename param; assert isa CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111568/new/ https://reviews.llvm.org/D111568 Files: clang/lib/Analysis/CFG.cpp clang/test/SemaCXX/switch-implicit-fallthrough.cpp clang/test

[PATCH] D111568: [clang] Omit most AttributedStatements from the CFG

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc74ab84ea23f: [clang] Omit most AttributedStatements from the CFG (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github

[PATCH] D111570: [clang/CFG] Don't explicitly add AttributedStmtClass to AlwaysAddList

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2cc42758a9d8: [clang/CFG] Don't explicitly add AttributedStmtClass to AlwaysAddList (authored by thakis). Herald added a project: clang. Repository:

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

thakis added a comment. This breaks tests on Mac: http://45.33.8.238/mac/37119/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

thakis added a comment. Thanks for this warning! I just finished turning it on in Chromium. Here's a short report on the things it fired on in case it's interesting to anyone. (I'm not suggesting any changes to the warning, I think it works very well as-is). The warning fired in 21 different fi

[PATCH] D112157: [clang] Disallow mixing SEH and Objective-C exceptions

thakis created this revision. thakis added a reviewer: rnk. thakis requested review of this revision. We already disallow mixing SEH and C++ exceptions, and mixing SEH and Objective-C exceptions seems to not work (see PR52233). Emitting an error is friendlier than crashing. https://reviews.llvm.

[PATCH] D112157: [clang] Disallow mixing SEH and Objective-C exceptions

This revision was automatically updated to reflect the committed changes. Closed by commit rGfb8829768d77: [clang] Disallow mixing SEH and Objective-C exceptions (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D112287: [clang] Implement CFG construction for @try and @catch

thakis created this revision. thakis added a reviewer: rnk. thakis requested review of this revision. @finally is still not implemented. With this, clang can emit -Wreturn-type warnings for functions containing @try/@catch (but not yet @finally), and -Wunreachable-code also works for those functi

[PATCH] D112287: [clang] Implement CFG construction for @try and @catch

thakis updated this revision to Diff 381450. thakis added a comment. clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112287/new/ https://reviews.llvm.org/D112287 Files: clang/lib/Analysis/CFG.cpp clang/test/Sema/warn-unreachable.m clang/test/Sema/warn-unreachable.mm

[PATCH] D99732: [AST] Pick last tentative definition as the acting definition

thakis added a comment. This breaks tests on macOS: http://45.33.8.238/macm1/16663/step_7.txt Please to a look. Just adding a -triple flag is probably sufficient. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99732/new/ https://reviews.llvm.org/D9

[PATCH] D108818: [clang] Add a -canonical-prefixes option

thakis created this revision. thakis added reviewers: hans, rnk. Herald added a subscriber: dang. thakis requested review of this revision. In https://reviews.llvm.org/D47480 I complained that there's no positive form of this flag, so let's add one :) https://gcc.gnu.org/PR29931 also has a pendin

[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

thakis added a comment. Herald added a subscriber: ormris. The test fails for me on macOS (normal cmake build): % tools/clang/unittests/Interpreter/ExceptionTests/ClangReplInterpreterExceptionTests [==] Running 1 test from 1 test suite. [--] Global test environment set-up.

[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

thakis added a comment. Also failing on the bots: https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/23343/execution/node/39/log/ (that's for the original land, but from what I can tell nothing has changed in this regard for the reland, and I'm still seeing the same failure

[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

thakis added a comment. > I did not find a good way to 'find back' the clang resource-dir and it seems > to be put in a constant place. The test needs to have a compiled and > interpreted parts -- it throws an exception from the interpreter land and > expects a compiled binary to be able to cat

[PATCH] D108818: [clang] Add a -canonical-prefixes option

This revision was automatically updated to reflect the committed changes. Closed by commit rG3d157cfcc4ef: [clang] Add a -canonical-prefixes option (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108

[PATCH] D109057: [openmp] Accept directory for libomptarget-bc-path

thakis added a comment. This breaks tests on windows: http://45.33.8.238/win/44930/step_7.txt Please take a look and revert if it takes a while to fix. Probably just need to optionally accept win slashiness in the test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D109085: clang/win: Add __readfsdword to intrin.h

thakis created this revision. thakis added a reviewer: hans. Herald added a subscriber: pengfei. thakis requested review of this revision. When using __readfsdword(), clang used to warn that one has to include -- no matter if that was already included or not. Now it only warns if it's not yet in

[PATCH] D109085: clang/win: Add __readfsdword to intrin.h

thakis updated this revision to Diff 370041. thakis added a comment. no-op rebase in the hope that the precommit bot doesn't run in a "git lockfile already held" error again CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109085/new/ https://reviews.llvm.org/D109085 Files: clang/lib/H

[PATCH] D109105: [clang-cl] Emit nicer warning on unknown /arch: arguments

thakis created this revision. thakis added a reviewer: hans. Herald added subscribers: dexonsmith, Jim, atanasyan, jrtc27, fedor.sergeev, sdardis, dylanmckay, emaste. thakis requested review of this revision. Herald added a project: LLVM. Now prints the list of known archs. This requires plumbing

[PATCH] D109105: [clang-cl] Emit nicer warning on unknown /arch: arguments

thakis updated this revision to Diff 370128. thakis added a comment. clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109105/new/ https://reviews.llvm.org/D109105 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/lib/Driver/ToolChains/AVR.cpp clang/lib/Dri

[PATCH] D109105: [clang-cl] Emit nicer warning on unknown /arch: arguments

thakis marked an inline comment as done. thakis added a comment. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109105/new/ https://reviews.llvm.org/D109105 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D109105: [clang-cl] Emit nicer warning on unknown /arch: arguments

This revision was automatically updated to reflect the committed changes. Closed by commit rG973519826edb: [clang-cl] Emit nicer warning on unknown /arch: arguments (authored by thakis). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D109105?vs=370128&id=370267

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

thakis added a comment. This seems to break tests: http://45.33.8.238/linux/54991/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108695/new/ https://reviews.llvm.org/D1086

[PATCH] D109085: clang/win: Add __readfsdword to intrin.h

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe5438f386854: clang/win: Add __readfsdword to intrin.h (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo C

[PATCH] D109167: Try to unbreak Win build differently after 973519826edb76

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5881dcff7e76: Try to unbreak Win build differently after 973519826edb76 (authored by thakis). Herald added a project: clang. Herald added a subscribe

[PATCH] D109167: Try to unbreak Win build differently after 973519826edb76

thakis added a comment. In D109167#2980560 , @GMNGeoffrey wrote: > I'm seeing a build failure coming out of this: > >> llvm-project/lld/wasm/Writer.cpp:521:16: error: non-const lvalue reference >> to type 'llvm::StringRef' cannot bind to a temporary of

[PATCH] D109181: Reland "Try to unbreak Win build differently after 973519826edb76""

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109181/new/ https://reviews.llvm.org/D109181

[PATCH] D109167: Try to unbreak Win build differently after 973519826edb76

thakis added a comment. In D109167#2980604 , @GMNGeoffrey wrote: >> I had fixed this in >> https://github.com/llvm/llvm-project/commit/9d227543890e721b95303430ee1427ce5aa7292f >> . Precommit bot on this change was green though as far as I can tell. > >

[PATCH] D109181: Reland "Try to unbreak Win build differently after 973519826edb76""

thakis added a comment. In D109181#2981093 , @GMNGeoffrey wrote: > Hmmm so pre-merge checks on Linux are failing > > https://buildkite.com/llvm-project/premerge-checks/builds/54955#8fea4a06-e971-48ad-a6d7-31173315d83d I can't repro this locally. I re-ra

[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

thakis added a comment. Thanks for fixing! Intel macs are happy now. But the test is still failing on arm macs: http://45.33.8.238/macm1/17292/step_7.txt FAIL: Clang-Unit :: Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException (28440 of 28440) ***

[PATCH] D109460: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

thakis added a comment. This is a nice idea. One drawback is that if you don't pass `-isysroot`, things just wouldn't work before but now every clang invocation will call xcrun, so things will work but will be slower than if you passed `-isysroot`. But the convenience for one-off invocations i

[PATCH] D99901: [Driver][test] Test intended target only

thakis added inline comments. Comment at: clang/test/Driver/nostdincxx.cpp:4 // RUN: not %clangxx -nostdlibinc %s 2>&1 | FileCheck %s -// RUN: not %clangxx -fsyntax-only -nostdinc -nostdinc++ %s 2>&1 | FileCheck /dev/null --implicit-check-not=-Wunused-command-line-argument +//

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

thakis added a comment. Can you just set `CLANG_TIDY_ENABLE_STATIC_ANALYZER=OFF` too if you care about this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109611/new/ https://reviews.llvm.org/D109611 __

[PATCH] D109624: [clang] Make the driver not diagnose errors on nonexistent linker inputs

This revision was automatically updated to reflect the committed changes. Closed by commit rG648feabc65d8: [clang] Make the driver not diagnose errors on nonexistent linker inputs (authored by thakis). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Gi

[PATCH] D109624: [clang] Make the driver not diagnose errors on nonexistent linker inputs

thakis added a comment. Hm, it passes on my win box and on the one running the presubmit test. Do you know if there's anything special about your bot? Any idea how I could reproduce this? The output is `error: command failed with exit status: True`. Repository: rG LLVM Github Monorepo CHANG

[PATCH] D109624: [clang] Make the driver not diagnose errors on nonexistent linker inputs

thakis added a comment. Does it fail consistently? Or was it a one-time flake? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109624/new/ https://reviews.llvm.org/D109624 ___ cfe-commits mailing list cfe-

[PATCH] D109624: [clang] Make the driver not diagnose errors on nonexistent linker inputs

thakis added a comment. In D109624#2998550 , @saghir wrote: > Hi, this change also breaks Power PC bots: > > 1. https://lab.llvm.org/buildbot/#/builders/19/builds/6451/steps/24/logs/stdio > 2. https://lab.llvm.org/buildbot/#/builders/18/builds/2443/steps/

[PATCH] D109624: [clang] Make the driver not diagnose errors on nonexistent linker inputs

thakis added a comment. In D109624#2998555 , @phosek wrote: > In D109624#2998419 , @thakis wrote: > >> Does it fail consistently? Or was it a one-time flake? > > It's been failing consistently. It might be related

[PATCH] D109624: [clang] Make the driver not diagnose errors on nonexistent linker inputs

thakis added a comment. In D109624#2998550 , @saghir wrote: > Hi, this change also breaks Power PC bots: > > 1. https://lab.llvm.org/buildbot/#/builders/19/builds/6451/steps/24/logs/stdio > 2. https://lab.llvm.org/buildbot/#/builders/18/builds/2443/steps/

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

thakis added a comment. This breaks tests: http://45.33.8.238/linux/55784/step_8.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108893/new/ https://reviews.llvm.org/D108893

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

thakis added a comment. Reverted in 76dc8ac36d07 for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108893/new/ https://reviews.llvm.org/D108893 ___

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

thakis added a comment. The test is still failing on Windows, see e.g. http://45.33.8.238/win/45205/step_8.txt or https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8836086236638553313/+/u/package_clang/stdout?format=raw or https://lab.llvm.org/buildbot/#/builders/123/builds/607

[PATCH] D109828: [clang-cl] Add a /diasdkdir flag and make /winsysroot imply it

thakis created this revision. thakis added a reviewer: hans. Herald added subscribers: dang, pengfei. thakis requested review of this revision. D109708 added "DIA SDK" to our win sysroot for hermetic builds that use LLVM_ENABLE_DIA_SDK. But the build system stil

[PATCH] D109828: [clang-cl] Add a /diasdkdir flag and make /winsysroot imply it

thakis updated this revision to Diff 372782. thakis marked an inline comment as done. thakis added a comment. move arch functions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109828/new/ https://reviews.llvm.org/D109828 Files: clang/include/clang/Driver/Options.td clang/lib/Driver

[PATCH] D109828: [clang-cl] Add a /diasdkdir flag and make /winsysroot imply it

thakis added a comment. In D109828#3002114 , @hans wrote: > The /winsysroot part makes sense to me, but what's the case for the new > /diasdkdir flag? - If you do have a vcvars shell, you don't need the full sysroot path and it's kind of useful (see

[PATCH] D109654: [clang] disable implicit moves when not in CPlusPLus

thakis added a comment. Is it possible that this disabled this for Objective-C++ too? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109654/new/ https://reviews.llvm.org/D109654 ___ cfe-commits mailing li

[PATCH] D109654: [clang] disable implicit moves when not in CPlusPLus

thakis added a comment. Yes, https://bugs.chromium.org/p/chromium/issues/detail?id=1250052 I haven't yet verified it's definitely due to this change, but it's the most related-looking change in the regression range. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D109654: [clang] disable implicit moves when not in CPlusPLus

thakis added a comment. I bet my issue is due to D108243 and not this change. Sorry for the noise! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109654/new/ https://reviews.llvm.org/D109654 _

<    15   16   17   18   19   20