[PATCH] D91861: [clang][cli] Split DefaultAnyOf into a default value and ImpliedByAnyOf

dexonsmith added a comment. I like this direction; the logic in the `.td` files seems much cleaner. The `MARSHALLING` macro logic seems a bit harder to follow and there may be a bug, but I'm not sure. See comments inline. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:

[PATCH] D91861: [clang][cli] Split DefaultAnyOf into a default value and ImpliedByAnyOf

dexonsmith added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4038-4043 if (((FLAGS)&options::CC1Option) && \ - (ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE)) { \ + (ALWAYS_E

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

aaron.ballman added a comment. In D89743#2406017 , @sammccall wrote: > In D89743#2363201 , @aaron.ballman > wrote: > >> In D89743#2360258 , @sammccall >> wrote: >> >>> (whi

[PATCH] D90507: [Driver] Add DWARF64 flag: -gdwarf64

ayermolo added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.def:35 CODEGENOPT(AsmVerbose, 1, 0) ///< -dA, -fverbose-asm. +CODEGENOPT(Dwarf64 , 1, 0) ///< -gdwarf64. CODEGENOPT(PreserveAsmComments, 1, 1) ///< -dA, -fno-preserve-as-comme

[PATCH] D91747: [Clang] Add __STDCPP_THREADS__ to standard predefine macros

rnk added inline comments. Comment at: clang/test/CXX/cpp/cpp.predefined/p2.cpp:1 +// RUN: %clang_cc1 %s -verify +// expected-no-diagnostics Let's expand on this: - test that we don't set the macro when compiling C (`-x c`) - test that we don't set the macro when

[PATCH] D91605: [sanitizers] Implement GetTls on Solaris

MaskRay added inline comments. Comment at: clang/tools/driver/CMakeLists.txt:123 + +check_linker_flag("-Wl,-z,relax=transtls" LINKER_SUPPORTS_Z_RELAX_TRANSTLS) ro wrote: > MaskRay wrote: > > ro wrote: > > > MaskRay wrote: > > > > GNU ld reports a warning instead

[PATCH] D89684: [AIX] Add mabi=vec-extabi options to enable the AIX extended and default vector ABIs.

Xiangling_L added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1443 + if (Arg *A = + Args.getLastArg(OPT_mabi_EQ_vec_default, OPT_mabi_EQ_vec_extabi)) { ZarkoCA wrote: > Xiangling_L wrote: > > Should we also check if target f

[clang] 2c7e24c - Guard init_priority attribute within libc++

Author: Zbigniew Sarbinowski Date: 2020-11-20T15:53:26-05:00 New Revision: 2c7e24c4b6893a93ddb2b2cca91eaf5bf7956965 URL: https://github.com/llvm/llvm-project/commit/2c7e24c4b6893a93ddb2b2cca91eaf5bf7956965 DIFF: https://github.com/llvm/llvm-project/commit/2c7e24c4b6893a93ddb2b2cca91eaf5bf795696

[PATCH] D91565: Guard init_priority attribute within libc++

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2c7e24c4b689: Guard init_priority attribute within libc++ (authore

[PATCH] D91844: [llvm][clang] Add checks for the smart pointers with the possibility to be null

dexonsmith added a comment. Is it possible to split these up into separate patches for unrelated code? Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:1346-1353 if (!Ptr) { // Search in reverse order so that the most-derived type is handled first. ArrayRef> B

[PATCH] D91893: [clang-tidy] performance-unnecessary-copy-initialization: Prevent false positives when dependent variable is modified.

flx created this revision. flx added reviewers: aaron.ballman, hokein. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. flx requested review of this revision. Extend the check to not only look at the variable the unnecessarily copied variable is initialized from, bu

[PATCH] D91893: [clang-tidy] performance-unnecessary-copy-initialization: Prevent false positives when dependent variable is modified.

flx updated this revision to Diff 306773. flx added a comment. Fix formatting and comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91893/new/ https://reviews.llvm.org/D91893 Files: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyI

[PATCH] D83694: [clang][cli] Port DependencyOutput option flags to new option parsing system

dexonsmith requested changes to this revision. dexonsmith added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Driver/Options.td:473 +MarshallingInfoFlag<"DependencyOutputOpts.OutputFormat", "DependencyOutputFormat::Make">, +

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

jfb added a comment. Let's make sure that we follow the same semantics that GCC does, particularly w.r.t. union, bitfields, and padding at the end of an object (whether it's in an array or not). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87974/

[PATCH] D83697: [clang][cli] Port Frontend option flags to new option parsing system

dexonsmith requested changes to this revision. dexonsmith added a comment. This revision now requires changes to proceed. This is close, just a few nits. Also, since this has the first uses of `IsNegative`, it'd be great to have a couple of tests for one of the flags it's used on. ===

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver added a comment. > Let's make sure that we follow the same semantics that GCC does, particularly > w.r.t. union, bitfields, and padding at the end of an object (whether it's in > an array or not). Agreed. I'm planning to run some tests tomorrow once the nightly build has updated. R

[PATCH] D91747: [Clang] Add __STDCPP_THREADS__ to standard predefine macros

zequanwu updated this revision to Diff 306777. zequanwu added a comment. Update tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91747/new/ https://reviews.llvm.org/D91747 Files: clang/include/clang/Basic/CodeGenOptions.h clang/include/cla

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

sammccall added a comment. OK, I'm going to drop hasAttrName from the patch (it's a relatively tiny piece of the code/tests) and land this, and put that back up as another review for discussion. And thanks, it's been interesting and I learned a bunch! We didn't talk about overloading isImplicit

[PATCH] D91747: [Clang] Add __STDCPP_THREADS__ to standard predefine macros

zequanwu marked an inline comment as done. zequanwu added inline comments. Comment at: clang/test/CXX/cpp/cpp.predefined/p2.cpp:1 +// RUN: %clang_cc1 %s -verify +// expected-no-diagnostics rnk wrote: > Let's expand on this: > - test that we don't set the macro wh

[PATCH] D91868: [clangd] Mention when CXXThis is implicit in exposed AST.

kadircet added a comment. This looks like an improvement to me as well, thanks! Comment at: clang-tools-extra/clangd/DumpAST.cpp:233 return CCO->getConstructor()->getNameAsString(); +if (const auto *CTE = dyn_cast(S)) { + bool Const = CTE->getType()->getPointeeT

[PATCH] D91821: Fix PR42049 - Crash when parsing bad decltype use within template argument list after name assumed to be a function template

rsmith added inline comments. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1055 + // semi-colon. + EndLoc = PP.getLastCachedTokenLocation(); +} It seems to me that either `EndLoc` was miscomputed and should already point to the last token that we s

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

__simt__ added a comment. I've resumed looking at the library code. How should I check for support? Is it going to be e.g. `__has_feature(__builtin_clear_padding)`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87974/new/ https://reviews.llvm.org

[PATCH] D91895: [Clang] avoid -Wimplicit-fallthrough for fallthrough to break

nickdesaulniers created this revision. nickdesaulniers added reviewers: rsmith, aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. nickdesaulniers requested review of this revision. Clang differs slightly than GCC for the pattern: switch (x) { case 0: ++x

[PATCH] D91895: [Clang] avoid -Wimplicit-fallthrough for fallthrough to break

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:1156 + const CFGBlock::AdjacentBlock &Succ = *P->succ_begin(); + const Stmt *Term = B.getTerminatorStmt(); + if (Succ->size() == 0 && Term && isa(Term))

[PATCH] D91895: [Clang] avoid -Wimplicit-fallthrough for fallthrough to break

nickdesaulniers updated this revision to Diff 306787. nickdesaulniers added a comment. - check the successor's terminator Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91895/new/ https://reviews.llvm.org/D91895 Files: clang/lib/Sema/AnalysisBase

[clang] 244022a - Don’t break before nested block param when prior param is not a block

Author: Samuel Giddins Date: 2020-11-20T15:16:04-08:00 New Revision: 244022a3cd75b51dcf1d2a5a822419492ce79e47 URL: https://github.com/llvm/llvm-project/commit/244022a3cd75b51dcf1d2a5a822419492ce79e47 DIFF: https://github.com/llvm/llvm-project/commit/244022a3cd75b51dcf1d2a5a822419492ce79e47.diff

[PATCH] D91669: Don’t break before nested block param when prior param is not a block

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG244022a3cd75: Don’t break before nested block param when prior param is not a block (authored by segiddins, committed by keith). Repository: rG LL

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

thejh updated this revision to Diff 306792. thejh marked an inline comment as done. thejh added a comment. As requested by @aaron.ballman, added tests for edgecases where sizeof()/typeid() can cause memory access. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

thejh added inline comments. Comment at: clang/test/Frontend/noderef.c:71 + x = sizeof(s->a + (s->b)); // ok + // Nested struct access aaron.ballman wrote: > Can you add tests for the weird situations where the expression actually is > evaluated? e.g., > ```

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver added a comment. > How should I check for support? Is it going to be e.g. > __has_feature(__builtin_clear_padding)? I'm not sure `__has_feature` will work but `__has_builtin` should work on both Clang and GCC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D83694: [clang][cli] Port DependencyOutput option flags to new option parsing system

dexonsmith added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:156-159 +template +FlagToValueNormalizer makeFlagToValueNormalizer(T Value) { + return FlagToValueNormalizer{std::move(Value)}; } dexonsmith wrote: > Please declare this `s

[PATCH] D83211: Factor out call to EXTRACTOR in generateCC1CommandLine

dexonsmith requested changes to this revision. dexonsmith added a comment. This revision now requires changes to proceed. Marking as "requested changes" to get this off my queue, but if my lambda suggestion doesn't work well for some reason (maybe it significantly increases compile-time), I'm op

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

sammccall added a comment. In D89743#2409001 , @sammccall wrote: > We didn't talk about overloading isImplicit to apply to attrs too, but it > doesn't seem like that was controversial (and it does help with the tests). I spoke too soon about this... This

[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

NoQ created this revision. NoQ added reviewers: aaron.ballman, dexonsmith, erik.pilkington, vsavchenko. Herald added subscribers: martong, Charusso, JDevlieghere, kristof.beyls. NoQ requested review of this revision. Patch by Sean Dooher! I'll be addressing the review comments. The point is to ma

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 306794. njames93 marked an inline comment as done. njames93 added a comment. Reworked everything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

sammccall updated this revision to Diff 306795. sammccall added a comment. Herald added subscribers: arphaman, kbarton, nemanjai. Drop hasAttrName matcher. Update uses of hasAncestor(isImplicit()) to hasAncestor(decl(isImplicit())), as the former no longer compiles due to lack of deduction. Impl

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 306796. njames93 marked 7 inline comments as done. njames93 added a comment. Messed up branches, fixed that Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: cl

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 marked 2 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clangd/TidyProvider.h:20 +/// The base class of all clang-tidy config providers for clangd. +class TidyProvider { +public: sammccall wrote: > we're using inhe

[PATCH] D91895: [Clang] avoid -Wimplicit-fallthrough for fallthrough to break

nickdesaulniers added a comment. Testing this on the kernel with `-Wimplicit-fallthrough`, there's some additional cases that are still warning: // mm/vmscan.c 1371 mapping = page_mapping(page);

[PATCH] D91868: [clangd] Mention when CXXThis is implicit in exposed AST.

sammccall added inline comments. Comment at: clang-tools-extra/clangd/DumpAST.cpp:233 return CCO->getConstructor()->getNameAsString(); +if (const auto *CTE = dyn_cast(S)) { + bool Const = CTE->getType()->getPointeeType().isLocalConstQualified(); k

[PATCH] D91859: [clangd] Fix shared-lib builds

kbobyrev added a comment. In D91859#2408064 , @sammccall wrote: > I'm not totally following the discussion on the more complete fix (LMK if you > want me to dig into that), but if the build is broken in some configurations > we should likely prioritize f

[PATCH] D91895: [Clang] avoid -Wimplicit-fallthrough for fallthrough to break

nickdesaulniers added a comment. And for `drivers/hid/usbhid/hid-core.c` it looks like multiple labels are modeled as individual blocks in the control flow graph. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91895/new/ https://reviews.llvm.org/D9

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

smhc updated this revision to Diff 306815. smhc added a comment. Updated after review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90282/new/ https://reviews.llvm.org/D90282 Files: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp clang-tools-extra/clang

[PATCH] D91904: [mac/arm] Fix rtti codegen tests when running on an arm mac

thakis created this revision. thakis added a reviewer: rjmccall. Herald added subscribers: kristof.beyls, dschuff. thakis requested review of this revision. Herald added a subscriber: aheejin. [mac/arm] Fix rtti codegen tests when running on an arm mac shouldRTTIBeUnique() returns false for iOS64

[clang] c473184 - [mac/arm] Fix clang/test/Sema/wchar.c on mac/arm hosts

Author: Nico Weber Date: 2020-11-20T21:49:17-05:00 New Revision: c47318491439642ed47cb9c9098333a8199fa54a URL: https://github.com/llvm/llvm-project/commit/c47318491439642ed47cb9c9098333a8199fa54a DIFF: https://github.com/llvm/llvm-project/commit/c47318491439642ed47cb9c9098333a8199fa54a.diff LO

[clang] e91b234 - [mac/arm] Fix test/Driver/darwin-sdk-version.c on arm macs

Author: Nico Weber Date: 2020-11-20T22:32:31-05:00 New Revision: e91b2344ad7294c47d8b10b3a84e962bc3ed4160 URL: https://github.com/llvm/llvm-project/commit/e91b2344ad7294c47d8b10b3a84e962bc3ed4160 DIFF: https://github.com/llvm/llvm-project/commit/e91b2344ad7294c47d8b10b3a84e962bc3ed4160.diff LO

[PATCH] D17993: [CodeGen] Apply 'nonnull' and 'dereferenceable(N)' to 'this' pointer arguments.

brooksmoses added a comment. So, I have bad news: This causes OpenJDK to segfault. The relevant code is here: https://github.com/openjdk/jdk/blob/master/src/hotspot/share/memory/arena.cpp#L311 void Arena::destruct_contents() { if (UseMallocOnly && _first != NULL) { char* end = _firs

<    1   2