[PATCH] D52849: [SEMA] split ExtWarn dupl-decl-spec's into Extension and ExtWarn

nickdesaulniers created this revision. nickdesaulniers added a reviewer: rsmith. Herald added a subscriber: cfe-commits. For types deduced from typedef's and typeof's, don't warn for duplicate declaration specifiers in C90 unless -pedantic. Create a third diagnostic type for duplicate declaration

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

nickdesaulniers added a comment. Created: https://reviews.llvm.org/D52849 Repository: rC Clang https://reviews.llvm.org/D52248 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52849: [SEMA] split ExtWarn dupl-decl-spec's into Extension and ExtWarn

This revision was automatically updated to reflect the committed changes. Closed by commit rL343740: [SEMA] split ExtWarn dupl-decl-spec's into Extension and ExtWarn (authored by nickdesaulniers, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.

[PATCH] D52849: [SEMA] split ExtWarn dupl-decl-spec's into Extension and ExtWarn

nickdesaulniers added a comment. Thanks for your suggestions and code review. I appreciate it. Repository: rL LLVM https://reviews.llvm.org/D52849 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listi

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

nickdesaulniers added inline comments. Comment at: lib/Sema/SemaExpr.cpp:15604 +public EvaluatedExprVisitor { + Sema &S; + ../tools/clang/lib/Sema/SemaExpr.cpp:15619:13: warning: private field 'S' is not used [-Wunused-private-field] Sema &S;

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

nickdesaulniers added inline comments. Comment at: lib/Sema/SemaExpr.cpp:15621-15623 +for (InitListExpr::iterator II = E->begin(), IE = E->end(); + II != IE; ++II) + Visit(*II); `std::for_each`? https://reviews.llvm.org/D52854 __

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

nickdesaulniers added inline comments. Comment at: lib/Sema/SemaExpr.cpp:15621-15623 +for (InitListExpr::iterator II = E->begin(), IE = E->end(); + II != IE; ++II) + Visit(*II); nickdesaulniers wrote: > `std::for_each`? Sorry, posted

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

nickdesaulniers added inline comments. Comment at: lib/Sema/SemaExpr.cpp:15621-15623 +for (InitListExpr::iterator II = E->begin(), IE = E->end(); + II != IE; ++II) + Visit(*II); void wrote: > nickdesaulniers wrote: > > nickdesaulniers

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

nickdesaulniers added inline comments. Comment at: test/Sema/builtins.c:125 +// __builtin_constant_p cannot resolve non-constants as a file scoped array. +int expr; such as Otherwise it's possible to read this as the non-constants being resolved to a file sco

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

nickdesaulniers added inline comments. Comment at: lib/Sema/SemaExpr.cpp:15621-15623 +for (InitListExpr::iterator II = E->begin(), IE = E->end(); + II != IE; ++II) + Visit(*II); nickdesaulniers wrote: > void wrote: > > nickdesaulniers

[PATCH] D53001: [DRIVER] check for exit code from SIGPIPE

nickdesaulniers created this revision. nickdesaulniers added a reviewer: jfb. Herald added a subscriber: cfe-commits. https://reviews.llvm.org/D53000 adds a special exit code for SIGPIPE (writing to a closed reader), and rather than print a fatal warning, skips printing the error. Fixes PR25349,

[PATCH] D53001: [Driver] check for exit code from SIGPIPE

nickdesaulniers added inline comments. Comment at: lib/Driver/Driver.cpp:1406 +// for SIGPIPE. Do not print diagnostics for this case. +if (Res == 71) + continue; jfb wrote: > jfb wrote: > > Ditto on magical number in a header. > I think you want to

[PATCH] D53001: [Driver] check for exit code from SIGPIPE

nickdesaulniers updated this revision to Diff 168728. nickdesaulniers added a comment. - prefer EX_IOERR from sysexits.h Repository: rC Clang https://reviews.llvm.org/D53001 Files: lib/Driver/Driver.cpp Index: lib/Driver/Driver.cpp

[PATCH] D53001: [Driver] check for exit code from SIGPIPE

nickdesaulniers updated this revision to Diff 168733. nickdesaulniers added a comment. - return error code 74 (EX_IOERR) if observed Repository: rC Clang https://reviews.llvm.org/D53001 Files: lib/Driver/Driver.cpp Index: lib/Driver/Driver.cpp

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

nickdesaulniers added inline comments. Comment at: test/CodeGen/asm.c:276 + return 1; +} I don't understand why, but this particular test case I cannot compile standalone without `-emit-llvm`. ``` int t32(int cond) { asm goto("testl %0, %0; jne %l1;" :: "r"(c

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: test/CodeGen/asm.c:276 + return 1; +} nickdesaulniers wrote: > I don't understand why, but this particular test case I cannot compile > standalone without `-emi

[PATCH] D58145: [Sema] Fix a bogus -Wconversion warning

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Thanks for fixing this regression (sorry, I caused it). Doesn't regress the tests I added. You'll probably want to request to Hans that this gets picked up for the 8.0 rele

[PATCH] D58145: [Sema] Fix a bogus -Wconversion warning

nickdesaulniers added a comment. > Fix this by just directly calling DiagnoseImpCast, since we don't really need > anything DiagnoseFloatingImpCast does anyways. Good catch; thanks for fixing and improving the tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58145/new/ https://rev

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

nickdesaulniers added a comment. Was this committed accidentally? Today in master I see: - r362106: Revert "clang support gnu asm goto." Erich Keane - r362059: Mark CodeGen/asm-goto.c as x86 specific after r362045 Fangrui Song - r362045: clang support gnu asm goto. Jennifer Yu and yet this

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

nickdesaulniers added inline comments. Comment at: clang-tools-extra/test/clang-tidy/linuxkernel-must-check-errs.c:6 +// Prototypes of the error functions. +void * __must_check ERR_PTR(long error); +long __must_check PTR_ERR(const void *ptr); Let's come up with

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

nickdesaulniers added a comment. For future travelers, we're tracking 2 related bugs: - `-no-integrated-as` providing different results: https://github.com/ClangBuiltLinux/linux/issues/500 - `IfConverter` `assert`ing on `INLINEASM_BR` `MachineInstr`s: https://github.com/ClangBuiltLinux/linux/is

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

nickdesaulniers added a comment. See also: https://github.com/ClangBuiltLinux/linux/issues/520 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59963/new/ https://reviews.llvm.org/D59963 ___ cfe-commits m

[PATCH] D63260: [Attr] Support _attribute__ ((fallthrough))

nickdesaulniers added a comment. Thanks for the patch, I look forward to this feature! I think the changes in `test/SemaCXX/warn-unused-label-error.cpp`, `test/Sema/block-literal.c`, and `test/Sema/address_spaces.c` should not be committed (2 look like unrelated cleanups?).

[PATCH] D63260: [Attr] Support _attribute__ ((fallthrough))

nickdesaulniers added inline comments. Comment at: lib/Sema/AnalysisBasedWarnings.cpp:1279 continue; - if (S.getLangOpts().CPlusPlus11) { + if (S.getLangOpts().CPlusPlus11 || S.getLangOpts().C99) { const Stmt *Term = B->getTerminatorStmt(); ---

[PATCH] D63260: [Attr] Support _attribute__ ((fallthrough))

nickdesaulniers added a comment. > but as part of this commit but *not* as part of this commit CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63260/new/ https://reviews.llvm.org/D63260 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D63260: [Attr] Support _attribute__ ((fallthrough))

nickdesaulniers added inline comments. Comment at: test/Sema/address_spaces.c:12 { - _AS2 *x;// expected-warning {{type specifier missing, defaults to 'int'}} + _AS2 *x;// expected-error {{use of undeclared identifier 'x'}} _AS1 float * _AS2 *B; efriedma wr

[PATCH] D63260: [Attr] Support _attribute__ ((fallthrough))

nickdesaulniers added inline comments. Comment at: test/Sema/address_spaces.c:12 { - _AS2 *x;// expected-warning {{type specifier missing, defaults to 'int'}} + _AS2 *x;// expected-error {{use of undeclared identifier 'x'}} _AS1 float * _AS2 *B; efriedma wr

[PATCH] D63260: [Attr] Support _attribute__ ((fallthrough))

nickdesaulniers added inline comments. Comment at: test/Sema/fallthrough-attr.c:48 + return n; +} Seems like lines 24-48 were an accidental copy+paste? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63260/new/ https://reviews.llvm.org/D63260 ___

[PATCH] D63369: [AST] Fixed extraneous warnings for binary conditional operator

nickdesaulniers added inline comments. Comment at: clang/test/Sema/warn-binary-conditional-expression-unused.c:8 +a ? : ++b; +a ? a : ++b; +++a ? : b; //expected-warning{{expression result unused}} Note to other reviewers: Whether or not we should wa

[PATCH] D63369: [AST] Fixed extraneous warnings for binary conditional operator

nickdesaulniers added inline comments. Comment at: clang/lib/AST/Expr.cpp:2351-2352 return false; if (!Exp->getLHS()) return true; return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx); rsmith wrote: > Looks like whoever wr

[PATCH] D63369: [AST] Fixed extraneous warnings for binary conditional operator

nickdesaulniers added inline comments. Comment at: clang/lib/AST/Expr.cpp:2348 // RHS are warnings. const ConditionalOperator *Exp = cast(this); +return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx) && `const auto *Exp` ==

[PATCH] D63369: [AST] Fixed extraneous warnings for binary conditional operator

nickdesaulniers accepted this revision. nickdesaulniers added a comment. Thanks for the patch and following up on the review comments. Let's work on getting you commit access now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63369/new/ https://r

[PATCH] D63533: [analyzer] Fix clang-tidy crash on GCCAsmStmt

nickdesaulniers added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:401 + case Stmt::GCCAsmStmtClass: +return; } NoQ wrote: > Please add a TODO to actually implement this functionality. And an `assert` that this `Stmt

[PATCH] D63533: [analyzer] Fix clang-tidy crash on GCCAsmStmt

nickdesaulniers added inline comments. Comment at: clang/test/Analysis/egraph-asm-goto-no-crash.cpp:1 +// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot %s +// RUN: cat %t.dot | FileCheck %s NoQ wrote: > NoQ wrote: > > NoQ wrote: > >

[PATCH] D63533: [analyzer] Fix clang-tidy crash on GCCAsmStmt

nickdesaulniers added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:401 + case Stmt::GCCAsmStmtClass: +return; } nickdesaulniers wrote: > NoQ wrote: > > Please add a TODO to actually implement this functionality. > An

[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling

nickdesaulniers added a comment. > This change breaks building the Linux kernel for arm32 (at least): Follow up discussion in https://github.com/ClangBuiltLinux/linux/issues/598. I think the kernel is wrong here. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D645

[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling

nickdesaulniers added a comment. ah, maybe the addition of `-no-integrated-as` shouldn't produce this error? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64527/new/ https://reviews.llvm.org/D64527 ___ cfe-commits mai

[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling

nickdesaulniers added inline comments. Comment at: cfe/trunk/lib/Driver/ToolChains/Clang.cpp:3555 + // in -E mode either for example, even though it's not really used either. + if (!isa(JA)) { +ArgStringList DummyArgs; Should you be checking `!TC.IsIntegrat

[PATCH] D64655: [Clang][Driver] don't error for unsupported as options for -no-integrates-as

nickdesaulniers created this revision. nickdesaulniers added a reviewer: rnk. Herald added a project: clang. Herald added a subscriber: cfe-commits. The Linux kernel's assembler flag detection started failing after r365703. It seems that Clang was erroring for -Wa, flags that it did not support (

[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling

nickdesaulniers added a comment. Fixup: https://reviews.llvm.org/D64655 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64527/new/ https://reviews.llvm.org/D64527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D64607: [clang-tidy] Fix crash on end location inside macro

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Thanks for the fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64607/new/ https://reviews.llvm.org/D64607 __

[PATCH] D64666: Allow Clang -Wconversion, -Wimplicit-float-conversion warns about integer type -> floating point type implicit conversion precision loss.

nickdesaulniers added a comment. Thanks for the patch @ziangwan ! Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3245 +def warn_impcast_integer_float_precision : Warning< + "implicit conversion from %0 to %1 may loses integer precision">, + InGroup, DefaultIgnor

[PATCH] D64666: Allow Clang -Wconversion, -Wimplicit-float-conversion warns about integer type -> floating point type implicit conversion precision loss.

nickdesaulniers added a comment. I also recommend re-titling the commit to something like: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64666/new/ https://reviews.llvm.org/D64666

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3245 +def warn_impcast_integer_float_precision : Warning< + "implicit conversion from %0 to %1 may loses integer precision">, + InGroup, DefaultIgnore; ziangwan

[PATCH] D64678: [Sema] Fix -Wuninitialized for struct assignment from GNU C statement expression

nickdesaulniers added a comment. Thanks for the patch. Minor nits looking for test cases that should have no warning (like the two examples provided in the bug). Comment at: clang/test/Sema/warn-uninitialized-statement-expression.c:7 + int i = ({ +int z = i; // expected-w

[PATCH] D64678: [Sema] Fix -Wuninitialized for struct assignment from GNU C statement expression

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:10890 +if (cast(OrigDecl)->getType()->isRecordType() && +dyn_cast_or_null(E)) { + return; Should just be `dyn_cast(E)`? http://llvm.org/docs/ProgrammersManual.html#the

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

nickdesaulniers added a comment. In D64666#1583792 , @xbolva00 wrote: > I had duplicated warning for C++11+ - my new warning and C++11’s narrowing > warning. Why would narrowing be C++11 specific? I would think that narrowing applies anywhere integrals

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. I verified the conversion values are correct in the precise warnings in the test cases. From that perspective, and the rest of the code LGTM. Please wait for review from an

[PATCH] D65108: Reland "driver: Don't warn about assembler flags being unused when not assembling"

nickdesaulniers added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2051 + // Claim flags for the integrated assembler only if it's being used. + if (IsIntegratedAs) { +if (UseRelaxAll(C, Args)) thakis wrote: > (This just wraps all the

[PATCH] D65108: Reland "driver: Don't warn about assembler flags being unused when not assembling"

nickdesaulniers added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2051 + // Claim flags for the integrated assembler only if it's being used. + if (IsIntegratedAs) { +if (UseRelaxAll(C, Args)) thakis wrote: > nickdesaulniers wrote: >

[PATCH] D63889: Check possible warnings on global initializers for reachability

nickdesaulniers added inline comments. Comment at: clang/include/clang/Sema/Sema.h:1689 + void popDeclForInitializer() { +DeclForInitializer.pop_back(); + } might be nice to return the result, but maybe YAGNI? Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D63889: Check possible warnings on global initializers for reachability

nickdesaulniers added inline comments. Comment at: clang/include/clang/Sema/Sema.h:1698 +} +return nullptr; + } Does: `return DeclForInitializer.empty() ? DeclForInitializer.back() : nullptr;` fit on one line? Comment at: clang/lib/Sem

[PATCH] D63889: Check possible warnings on global initializers for reachability

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2009 +AnalysisDeclContext &AC, +SmallVector PUDs) { + is `clang` namespace required here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D64678: [Sema] Fix -Wuninitialized for struct assignment from GNU C statement expression

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Thanks for the patch and following up on the review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64678/new/ https://reviews.

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. In D59963#195 , @dvyukov wrote: > Re more checks. I guess we can borrow some from the existing checkers: > https://www.kernel.org

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

nickdesaulniers added a comment. In D64454#1600922 , @aaron.ballman wrote: > I use svn in-tree, so I tried it out and it does not seem to work for me. > > c:\llvm\tools\clang\tools\extra\docs\clang-tidy\checks>python > gen-static-analyzer-docs.py > T

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

nickdesaulniers added inline comments. Comment at: clang/lib/CodeGen/CGStmt.cpp:1989 std::vector Args; + std::vector ResultTypeRequiresCast; Are we able to use something other than `std::vector` here from ADT? http://llvm.org/docs/ProgrammersManual.html#bit

[PATCH] D65184: [Sema] Thread Safety Analysis: Fix negative capability's LockKind representation.

nickdesaulniers added inline comments. Comment at: clang/lib/Analysis/ThreadSafety.cpp:2219-2221 +if (LDat1->kind() == LK_Generic || LDat2->kind() == LK_Generic) { + // No warning is issued in this case. + if (Modify && LDat1->kind() == LK_Generic) { ---

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

nickdesaulniers requested changes to this revision. nickdesaulniers added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:12 +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include + ---

[PATCH] D65302: [clang][docs][release notes] mention asm goto support

nickdesaulniers created this revision. nickdesaulniers added reviewers: hans, tstellar. Herald added a project: clang. Herald added a subscriber: cfe-commits. nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: clang/docs/ReleaseNo

[PATCH] D65302: [clang][docs][release notes] mention asm goto support

nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: clang/docs/ReleaseNotes.rst:116 + control flow from inline assembly. The main consumer of this construct is the + Linux kernel and glib. There a few long tail bugs in Clang's in

[PATCH] D65302: [clang][docs][release notes] mention asm goto support

nickdesaulniers updated this revision to Diff 212002. nickdesaulniers added a comment. Herald added a reviewer: alexshap. - add details about ClangBuiltLinux Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65302/new/ https://reviews.llvm.org/D65302

[PATCH] D65302: [clang][docs][release notes] mention asm goto support

nickdesaulniers marked an inline comment as done. nickdesaulniers added a comment. Thanks for the tips, how does that look @hans ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65302/new/ https://reviews.llvm.org/D65302 _

[PATCH] D65302: [clang][docs][release notes] mention asm goto support

nickdesaulniers added a comment. > Go ahead and commit directly to the branch (I think it can only be done with > SVN still), or let me know if you'd like me to do it. Thanks again for the review and tips. I was going to ask how does committing work now... I don't have an SVN checkout; would

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

nickdesaulniers added a comment. Thanks so much for this patch! I look forward to support for `__gnu_mcount` for the arm32 Linux kernel. > What a horrible function. AAPCS? Who cares about that? haha Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1944 + + // Re

[PATCH] D65549: [Sema] Prevent -Wunused-lambda-capture from generating false positive warnings on templated member function

nickdesaulniers added a comment. Thanks for the patch @ziangwan ! Comment at: clang/lib/Sema/SemaExpr.cpp:5631 + } +} + LLVM style is not to use `{}` for single statement bodies. Repository: rL LLVM CHANGES SINCE LAST ACTION https://revi

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

nickdesaulniers added inline comments. Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:211 unsigned ARMMoveToIntReg(MVT VT, unsigned SrcReg); -unsigned ARMSelectCallOp(bool UseReg); +unsigned ARMSelectCallOp(bool UseReg, bool PushLR = false); unsigned ARMLowerPI

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:325 + ? "llvm.arm.gnu.eabi.mcount" : "\01mcount"; Doesn't require changes,

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

nickdesaulniers added inline comments. Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:12 +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include + tmroeder wrote: > tmroeder wrote: > > nickdesaulniers wrote: > > > tmroeder wrote: >

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

nickdesaulniers added inline comments. Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:3485 + cast( + Op.getOperand(Op.getOperand(0).getValueType() == MVT::Other ? 1 : 0)) + ->getZExtValue(); `Op.getOperand(0).getValueType() == MVT::Oth

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

nickdesaulniers added inline comments. Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1931 + +// bl__gnu_mcount_nc +MIB = BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(ARM::tBL)); should there be a space in this comment (and the one on l

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

nickdesaulniers added inline comments. Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1922 + const bool Thumb = Opcode == ARM::tBL_PUSHLR; + MachineOperand ReturnAddr = MI.getOperand(0); + assert(ReturnAddr.getReg() == ARM::LR && "expect LR register!"); -

[PATCH] D66186: Fix warning on printf('%hd', [char])

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. LGTM thanks for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66186/new/ https://reviews.llvm.org/D66186 ___

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

nickdesaulniers accepted this revision. nickdesaulniers added a subscriber: eli.friedman. nickdesaulniers added a comment. This revision is now accepted and ready to land. Great! LGTM and thank you for this patch. Please give 24hrs for @eli.friedman or @kristof.beyls to leave comments before mer

[PATCH] D63533: [analyzer] Fix clang-tidy crash on GCCAsmStmt

nickdesaulniers added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:401 + case Stmt::GCCAsmStmtClass: +assert(cast(Term)->isAsmGoto() == true && "Encountered GCCAsmStmt without labels"); +// TODO: Handle jumping to labels

[PATCH] D63889: Check possible warnings on global initializers for reachability

nickdesaulniers added inline comments. Comment at: clang/include/clang/Sema/AnalysisBasedWarnings.h:101 + void RegisterVarDeclWarning(VarDecl *VD, PossiblyUnreachableDiag + PossiblyUnreachableDiag); + `git-clang-format HEAD~` The formal pa

[PATCH] D63889: Check possible warnings on global initializers for reachability

nickdesaulniers added inline comments. Comment at: clang/include/clang/Sema/AnalysisBasedWarnings.h:95 + void flushDiagnostics(SmallVector); + Methods should be UpperCamelCased. Comment at: clang/include/clang/Sema/AnalysisBasedWarnings.h:11

[PATCH] D62116: [Sema] raise nullptr check to cover additional uses

nickdesaulniers created this revision. nickdesaulniers added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. This was flagged in https://www.viva64.com/en/b/0629/ under "Snippet No. 14" (see under #13). It looks like PVS studio flags nullptr checks where

[PATCH] D62116: [Sema] raise nullptr check to cover additional uses

nickdesaulniers added a comment. Note that this changes when the destructor for `CXXThisScopeRAII` runs. It's not clear to me why `ThisScope` is constructed at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62116/new/ https://reviews.llvm.org

[PATCH] D62116: [Sema] raise nullptr check to cover additional uses

nickdesaulniers added a comment. A similar case exists and was flagged in https://www.viva64.com/en/b/0629/ under "Snippet No. 16" (see under #13), `clang/lib/Sema/SemaTemplateInstantiate.cpp` `Sema::InstantiateClass()`. I'll wait for feedback on this patch, then either roll up the additional

[PATCH] D57930: [Driver] Verify GCCInstallation is valid

nickdesaulniers added a comment. @danielmentz thanks for this patch! Some teammates are running into issues that would be fixed by this patch. I think @srhines is asking for a test similar to what I wrote in r344941 (https://github.com/llvm/llvm-project/commit/11dadac247e677de124328077d080fe086

[PATCH] D57930: [Driver] Verify GCCInstallation is valid

nickdesaulniers updated this revision to Diff 200580. nickdesaulniers added a comment. Herald added a subscriber: ormris. - add unit test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57930/new/ https://reviews.llvm.org/D57930 Files: clang/lib/D

[PATCH] D57930: [Driver] Verify GCCInstallation is valid

This revision was automatically updated to reflect the committed changes. Closed by commit rC361314: [Driver] Verify GCCInstallation is valid (authored by nickdesaulniers, committed by ). Changed prior to commit: https://reviews.llvm.org/D57930?vs=200580&id=200586#toc Repository: rC Clang C

[PATCH] D62116: [Sema] raise nullptr check to cover additional uses

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:557-558 +dyn_cast_or_null(ND->getDeclContext()); +CXXThisScopeRAII ThisScope(*this, ThisContext, Qualifiers(), + ND->isCXXInstan

[PATCH] D62230: [CGDebugInfo] return early on failed dyn_cast

nickdesaulniers created this revision. nickdesaulniers added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. nickdesaulniers added a comment. Also of note is that no existing test covers this case. This was flagged in https://www.viva64.com/en/b/0629/ u

[PATCH] D62230: [CGDebugInfo] return early on failed dyn_cast

nickdesaulniers added a comment. Also of note is that no existing test covers this case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62230/new/ https://reviews.llvm.org/D62230 ___ cfe-commits mailing

[PATCH] D62232: [Clang][Driver] recheck for nullptr

nickdesaulniers created this revision. nickdesaulniers added reviewers: rsmith, sfantao. Herald added a project: clang. Herald added a subscriber: cfe-commits. This was flagged in https://www.viva64.com/en/b/0629/ under "Snippet No. 47". Looks like TC is checked for nullptr once, but not the seco

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

nickdesaulniers accepted this revision. nickdesaulniers added a comment. Ok, from the Linux kernel's perspective, I believe we have worked out all underlying issues in LLVM wrt callbr that would prevent us from using asm goto successfully. This patch now has my blessing; thanks for all the hard

[PATCH] D54188: [Sema] Mark target of __attribute__((alias("target"))) used for C

nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: test/Sema/alias-unused.c:3 +// expected-no-diagnostics +int f() { return 42; } +int g() __attribute__((alias("f"))); rjmccall wrote: > Is this meant to be `static

[PATCH] D54188: [Sema] Mark target of __attribute__((alias("target"))) used for C

nickdesaulniers updated this revision to Diff 180932. nickdesaulniers added a comment. - add static keyword to test case Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54188/new/ https://reviews.llvm.org/D54188 Files: lib/Sema/SemaDeclAttr.cpp test/Sema/alias

[PATCH] D54188: [Sema] Mark target of __attribute__((alias("target"))) used for C

nickdesaulniers marked 2 inline comments as done. nickdesaulniers added inline comments. Comment at: test/Sema/alias-unused.c:3 +// expected-no-diagnostics +int f() { return 42; } +int g() __attribute__((alias("f"))); rjmccall wrote: > nickdesaulniers wrote: > >

[PATCH] D54188: [Sema] Mark target of __attribute__((alias("target"))) used for C

This revision was automatically updated to reflect the committed changes. nickdesaulniers marked an inline comment as done. Closed by commit rL350776: [Sema] Mark target of __attribute__((alias("target"))) used for C (authored by nickdesaulniers, committed by ). Herald added a subscriber: llvm-co

[PATCH] D56522: [SemaCXX] add -Woverride-init alias to -Winitializer-overrides

nickdesaulniers created this revision. nickdesaulniers added a reviewer: rsmith. Herald added a subscriber: cfe-commits. https://bugs.llvm.org/show_bug.cgi?id=40251 https://github.com/ClangBuiltLinux/linux/issues/307 Repository: rC Clang https://reviews.llvm.org/D56522 Files: docs/Diagnost

[PATCH] D56522: [SemaCXX] add -Woverride-init alias to -Winitializer-overrides

nickdesaulniers updated this revision to Diff 181108. nickdesaulniers added a comment. - add gcc compat comment Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56522/new/ https://reviews.llvm.org/D56522 Files: docs/DiagnosticsReference.rst include/clang/Basic/

[PATCH] D56522: [SemaCXX] add -Woverride-init alias to -Winitializer-overrides

This revision was automatically updated to reflect the committed changes. Closed by commit rL350877: [SemaCXX] add -Woverride-init alias to -Winitializer-overrides (authored by nickdesaulniers, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTI

[PATCH] D67455: [Clang][CodeGen] support alias attribute w/ gnu_inline

nickdesaulniers created this revision. nickdesaulniers added reviewers: aaron.ballman, rsmith, erichkeane. Herald added a project: clang. Herald added a subscriber: cfe-commits. r369705 did not consider the addition of gnu_inline on function declarations of alias attributed functions. This resulte

[PATCH] D67455: [Clang][CodeGen] support alias attribute w/ gnu_inline

nickdesaulniers updated this revision to Diff 219790. nickdesaulniers added a comment. - adjust parens Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67455/new/ https://reviews.llvm.org/D67455 Files: clang/lib/AST/Decl.cpp clang/test/CodeGen/al

[PATCH] D67455: [Clang][CodeGen] support alias attribute w/ gnu_inline

This revision was automatically updated to reflect the committed changes. Closed by commit rL371766: [Clang][CodeGen] support alias attribute w/ gnu_inline (authored by nickdesaulniers, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode

nickdesaulniers added inline comments. Comment at: lib/AST/Decl.cpp:3283 if (Context.getLangOpts().CPlusPlus) return false; I would have thought the existing case here would handle your change. If it doesn't, why not? Should your change also remove t

[PATCH] D68410: [AttrDocs] document always_inline

nickdesaulniers created this revision. nickdesaulniers added reviewers: chandlerc, rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. GNU documentaion for always_inline: https://gcc.gnu.org/onlinedocs/gcc/Inline.html GNU documentation for function attributes: https://g

<    1   2   3   4   5   6   7   8   9   10   >