[PATCH] D150001: [clang] Fix initializer_list matching failures with modules

inclyc accepted this revision. inclyc added a comment. This revision is now accepted and ready to land. I think this fix is simple and straightforward and resolved the problem mentioned on GH. However, please wait for #clang-language-wg member's

[PATCH] D143439: [RISCV] Add vendor-defined XTheadBb (basic bit-manipulation) extension

inclyc added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:320 +if (Subtarget.is64Bit()) + setOperationAction({ISD::CTLZ, ISD::CTLZ_ZERO_UNDEF}, MVT::i32, Custom); + } inclyc wrote: > And this one Ah, this is my fault :(. Ther

[PATCH] D143439: [RISCV] Add vendor-defined XTheadBb (basic bit-manipulation) extension

inclyc added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:315 + if (Subtarget.hasVendorXTHeadBb()) { +setOperationAction({ISD::CTLZ}, XLenVT, Legal); + It looks like this line of code will cause compilation warning. ``` [1677/171

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc added a comment. On Mon, Jan 16, 2023 Roman Lebedev wrote: > Reminder to please always mention the reason for the revert/recommit in the > commit message. Thanks! This change needs to be fixed up (see comments above). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D133574: [C2x] reject type definitions in offsetof

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe327b52766ed: [C2x] reject type definitions in offsetof (authored by inclyc). Changed prior to commit: https://reviews.llvm.org/D133574?vs=482719&

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc added a comment. ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133574/new/ https://reviews.llvm.org/D133574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc added a comment. ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133574/new/ https://reviews.llvm.org/D133574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc added a comment. Hi @aaron.ballman seems we are failing on recent change 2fc5a3410087c209567c7e4a2c457b6896c127a3 /* The DR asked a question about whether defining a new type within offsetof * is allowed. C2x N235

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc updated this revision to Diff 482719. inclyc added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133574/new/ https://reviews.llvm.org/D133574 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaK

[PATCH] D137343: [clang] add -Wvla-stack-allocation

inclyc added a comment. ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137343/new/ https://reviews.llvm.org/D137343 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

inclyc abandoned this revision. inclyc added a comment. Prefer https://reviews.llvm.org/D137343 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132952/new/ https://reviews.llvm.org/D132952 ___ cfe-commits

[PATCH] D137343: [clang] add -Wvla-stack-allocation

inclyc added a comment. We may need a release note here so users know about the new changes to -Wvla. See `clang/docs/ReleaseNotes.rst`! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137343/new/ https://reviews.llvm.org/D137343 __

[PATCH] D137839: [Sema] check InitListExpr format strings like {"foo"}

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2ec79afd8993: [Sema] check InitListExpr format strings like {"foo"} (authored by inclyc). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D137839: [Sema] check InitListExpr format strings like {"foo"}

inclyc updated this revision to Diff 477279. inclyc added a comment. Address comments - add a test to coverage warnings if appropriate Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137839/new/ https://reviews.llvm.org/D137839 Files: clang/lib/S

[PATCH] D137839: [Sema] check InitListExpr format strings like {"foo"}

inclyc added a comment. Do we need release notes here? This patch is just an improvement to https://reviews.llvm.org/D130906 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137839/new/ https://reviews.llvm.org/D137839 __

[PATCH] D137839: [Sema] check InitListExpr format strings like {"foo"}

inclyc added a comment. ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137839/new/ https://reviews.llvm.org/D137839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D137839: [Sema] check InitListExpr format strings like {"foo"}

inclyc created this revision. Herald added a project: All. inclyc added a reviewer: aaron.ballman. inclyc published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits. Adds InitListExpr case in format string checks. e.g. int sprintf(char *__restrict

[PATCH] D137043: [clang] add implicit include for Linux/gnu compatibility

inclyc added a comment. > That is OK for glibc system because glibc includes this file manually. But > with other libc (e.g. musl), which does not manually include this, clang will > fail but GCC can get it compiled. FWIW currently we cannot build libedit using clang on x86_64-unknown-linux-mus

[PATCH] D133248: [clang] Fix crash upon stray coloncolon token in C2x mode

inclyc added a comment. In D133248#3865010 , @SuibianP wrote: > @aaron.ballman Thanks for the guidance! I have rectified the parentheses and > appended to the release notes. Please feel free to point out any additional > issues with the Differential. >

[PATCH] D133248: [clang] Fix crash upon stray coloncolon token in C2x mode

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG94e8bd002c81: [clang] Fix crash upon stray coloncolon token in C2x mode (authored by Jialun Hu , committed by inclyc). Reposit

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

inclyc added a comment. This makes sense! However I think `assert(0)` should not be used in this case, we could expose another `llvm_unreachable`-like api and probably `llvm_report_error` shall be fine. Are there some changed assertions actually "Aspirationally unreachable" in this patch? Rep

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

inclyc added a comment. In D135551#3849983 , @aaron.ballman wrote: > In D135551#3849944 , @inclyc wrote: > >>> - Prefer llvm_report_error() in any circumstance under which a code path is >>> functionally possibl

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

inclyc added a comment. > - Prefer llvm_report_error() in any circumstance under which a code path is > functionally possible to reach, but only in erroneous executions that signify > a mistake on the part of the LLVM developer elsewhere in the program. > - Prefer llvm_unreachable() in any circu

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

inclyc added a comment. > I've left some comments in the review about examples of my concerns (it's not > an exhaustive review). Thanks @aaron.ballman ! I didn't quite understand the original meaning of this code here (e.g. libclang), and I have now removed the relevant changes. I think this p

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

inclyc updated this revision to Diff 466539. inclyc added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135551/new/ https://reviews.llvm.org/D135551 Files: clang/include/clang/AST/Redeclarable.h clang/include/clang/

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

inclyc added a comment. In D135551#3846592 , @aaron.ballman wrote: > I don't know if that discussion reached a conclusion to move forward with > this change -- my reading of the conversation was that efforts would be > better spend on fuzzing instead o

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

inclyc updated this revision to Diff 466471. inclyc added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix CI issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135551/new/ https://reviews.llvm.org/D135551 Fil

[PATCH] D134815: [Sema] print more readable identifier of anonymous struct of -Wconsumed

inclyc created this revision. Herald added a reviewer: aaron.ballman. Herald added a project: All. inclyc published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits. Working in D133574 we discovered -Wconsumed print

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc updated this revision to Diff 463063. inclyc added a comment. This revision fixes: **anonymous struct** > You should be able to pass in the TagDecl directly because the diagnostics > engine knows how to print a NamedDecl. I've switch back to using `Context.getTagDeclType(New)` because th

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc updated this revision to Diff 462942. inclyc added a comment. Fix comment nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133574/new/ https://reviews.llvm.org/D133574 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/Diagn

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc updated this revision to Diff 462940. inclyc added a comment. Address comments from @aaron.ballman Move `OffsetOfKind` to `Sema` and pass it to `Sema:ActOnTag` directly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133574/new/ https://revi

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc added a comment. related kernel patch https://lkml.org/lkml/2022/9/26/1484 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133574/new/ https://reviews.llvm.org/D133574 ___ cfe-commits mailing list c

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc added inline comments. Comment at: clang/include/clang/Sema/Sema.h:3279 bool IsTypeSpecifier, bool IsTemplateParamOrArg, + bool IsWithinOffsetOf, bool IsOffsetOfInMacro, SkipBodyInfo *SkipBody = nullptr);

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc added a comment. Hi @aaron.ballman, I've noticed in the linux kernel, type alignment was implemented by a tricky way using offsetof. #define TYPE_ALIGN(TYPE) offsetof(struct { char x; TYPE test; }, test) Does this always has the same semantic with C11 `_Alignof`? If this is not true,

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc updated this revision to Diff 462680. inclyc added a comment. Address comments. Clang will now consider __builtin_offsetof #defined from "offsetof" to improve diagnostic message. For example: #define offsetof(t, d) __builtin_offsetof(t, d) int main() { return offsetof(struct S

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1650 +def err_type_defined_in_offsetof : Error< + "%0 cannot be defined in '__builtin_offsetof'">; aaron.ballman wrote: > inclyc wrote: > > aaron.ballman wrote: > > > We

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc added a comment. Emm, is it necessary to add a `LangOpts` check so that this change only applies to c2x? If clang was invoked without `-std=c2x`, should we just accept `offsetof` with definitions? Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1650 +def e

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc updated this revision to Diff 461822. inclyc added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133574/new/ https://reviews.llvm.org/D133574 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaK

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc updated this revision to Diff 461463. inclyc added a comment. Switch back to RAIIObject. Currently clang could not generate diagnostic messages for nested definitions in C++. I believe using RAIIObject here is logically correct, but in C++ mode, `ActOnTag` returns `nullptr`, and the commen

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc added inline comments. Comment at: clang/test/Sema/offsetof.c:79 +int a; +struct B // no-error, struct B is not defined within __builtin_offsetof directly +{ aaron.ballman wrote: > inclyc wrote: > > inclyc wrote: > > > aaron.ballman wrote: > >

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc added inline comments. Comment at: clang/test/Sema/offsetof.c:79 +int a; +struct B // no-error, struct B is not defined within __builtin_offsetof directly +{ inclyc wrote: > aaron.ballman wrote: > > inclyc wrote: > > > aaron.ballman wrote: > >

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc added inline comments. Comment at: clang/test/Sema/offsetof.c:79 +int a; +struct B // no-error, struct B is not defined within __builtin_offsetof directly +{ aaron.ballman wrote: > inclyc wrote: > > aaron.ballman wrote: > > > I think this is d

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc added inline comments. Comment at: clang/include/clang/Parse/Parser.h:2311 case DeclSpecContext::DSC_association: +case DeclSpecContext::DSC_offsetof: return true; aaron.ballman wrote: > Is this correct? I don't think we can deduce the type

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc updated this revision to Diff 459634. inclyc added a comment. git-clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133574/new/ https://reviews.llvm.org/D133574 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/Diagn

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc updated this revision to Diff 459633. inclyc added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133574/new/ https://reviews.llvm.org/D133574 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/Diagn

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc updated this revision to Diff 459458. inclyc added a comment. Use declaration context Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133574/new/ https://reviews.llvm.org/D133574 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basi

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc updated this revision to Diff 459248. inclyc added a comment. Use RAII object to maintain the Parser state > have you explored making a new DeclSpecContext and modifying > isDefiningTypeSpecifierContext()? I think that would likely be a cleaner > approach. Emm, I've tried passing a Decl

[PATCH] D133609: [Sema] compat warning of using deduced type in non-type template parameter

inclyc added inline comments. Comment at: clang/test/SemaCXX/template-nontype-args-compat.cpp:1 +// RUN: %clang_cc1 -fsyntax-only -Wpre-c++20-compat -std=c++20 -verify=cpp20 %s + I cannot find other tests of compatibility warnings though. Is it necessary to ad

[PATCH] D133609: [Sema] compat warning of using deduced type in non-type template parameter

inclyc created this revision. Herald added a subscriber: kristof.beyls. Herald added a project: All. inclyc added reviewers: aaron.ballman, mizvekov, clang-language-wg. inclyc published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits. Seems we are mi

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc updated this revision to Diff 459039. inclyc added a comment. Use double backquotes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133574/new/ https://reviews.llvm.org/D133574 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc updated this revision to Diff 459037. inclyc added a comment. Add release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133574/new/ https://reviews.llvm.org/D133574 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/Diag

[PATCH] D133574: [C2x] reject type definitions in offsetof

inclyc created this revision. Herald added a project: All. inclyc updated this revision to Diff 459034. inclyc added a comment. inclyc added reviewers: aaron.ballman, clang-language-wg. inclyc published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

inclyc added a comment. > Yeah, that's a different way of delineating than I was thinking originally > and it's worth more thought. I was thinking the separation would be "this is > a VLA" (for people who want to avoid all VLA stack allocations due to the > security concerns) and "this is a por

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

inclyc added inline comments. Comment at: clang/test/Sema/warn-vla.c:8-12 +void test2(int n, int v[n]) { // c99 no-warning +#if __STDC_VERSION__ < 199901L +// expected-warning@-2{{variable length arrays are a C99 feature}} +#endif } aaron.ballman wrote: > The di

[PATCH] D133248: [clang] Fix crash upon stray coloncolon token in C2x mode

inclyc added inline comments. Comment at: clang/lib/Parse/ParseDecl.cpp:5340 +if (!getLangOpts().CPlusPlus) + return false; if (NextToken().is(tok::kw_new) ||// ::new Maybe we can make a new `error` diagnostic definition and fire that here? ==

[PATCH] D133197: [clang] Fix crash when parsing scanf format string with missing arguments

inclyc added a reviewer: aaron.ballman. inclyc accepted this revision. inclyc added a subscriber: aaron.ballman. inclyc added a comment. This revision is now accepted and ready to land. Thanks @serge-sans-paille! Basically LGTM. Maybe we need to backport this patch though? @aaron.ballman Reposi

[PATCH] D133085: [clang] trim trailing space in format tests. NFC

inclyc added a comment. Thank you for your patience and detailed explanation! Sorry for waste your time though ( Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133085/new/ https://reviews.llvm.org/D133085 __

[PATCH] D133085: [clang] trim trailing space in format tests. NFC

inclyc added a comment. In D133085#3763198 , @ChuanqiXu wrote: > Do you have commit access? If you have, I remember LLVM encourages to land > such fixes directly without reviewed. (+ @aaron.ballman to make sure) Thanks! I'm just not sure whether these c

[PATCH] D133085: [clang] trim trailing space in format tests. NFC

This revision was automatically updated to reflect the committed changes. Closed by commit rG5ee51e815425: [clang] trim trailing space in format tests. NFC (authored by inclyc). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133085/new/ https://revie

[PATCH] D133085: [clang] trim trailing space in format tests. NFC

inclyc created this revision. inclyc added a reviewer: clang-language-wg. Herald added a project: All. inclyc requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Found in https://reviews.llvm.org/D132568 Repository: rG LLVM Github Monorepo

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe3bd67eddf65: [clang][Sema] check default argument promotions for printf (authored by inclyc). Changed prior to commit: https://reviews.llvm.org/D

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc updated this revision to Diff 457153. inclyc added a comment. Update comments according to review feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132568/new/ https://reviews.llvm.org/D132568 Files: clang/docs/ReleaseNotes.rst cla

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

inclyc added inline comments. Comment at: clang/include/clang/Sema/Sema.h:1760 +// Special builder emitting no diagnostics +SemaDiagnosticBuilder(Sema &S) : S(S) {} SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID, erichkeane wrote:

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

inclyc created this revision. inclyc added a reviewer: aaron.ballman. Herald added a project: All. inclyc added reviewers: rsmith, clang-language-wg. inclyc published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes: https://github.com/llvm/llv

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc updated this revision to Diff 456111. inclyc added a comment. Delete extra newline. Currently this patch has not fully implemented `wchar_t` related support, this type seems to be even platform dependent in C language, if possible, maybe we can consider support in subsequent patches? Rep

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc updated this revision to Diff 455898. inclyc added a comment. Update comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132568/new/ https://reviews.llvm.org/D132568 Files: clang/docs/ReleaseNotes.rst clang/include/clang/AST/FormatSt

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc added inline comments. Comment at: clang/lib/AST/FormatString.cpp:367 +case BuiltinType::Char_U: +case BuiltinType::Bool: + return Match; aaron.ballman wrote: > inclyc wrote: > > aaron.ballman wrote: > > > Isn't this a match promot

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc updated this revision to Diff 455871. inclyc added a comment. Add LangOpts so we keep the Obj-C behavior of clang just as-is. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132568/new/ https://reviews.llvm.org/D132568 Files: clang/docs/Rel

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc updated this revision to Diff 455793. inclyc added a comment. Trim whitespace in format.mm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132568/new/ https://reviews.llvm.org/D132568 Files: clang/docs/ReleaseNotes.rst clang/include/clang

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:10127-10129 +if (ImplicitMatch != ArgType::NoMatchPromotionTypeConfusion && +ImplicitMatch != ArgType::NoMatchTypeConfusion && +!IsCharacterLiteralInt) nickdesaulniers w

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc updated this revision to Diff 455671. inclyc added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132568/new/ https://reviews.llvm.org/D132568 Files: clang/docs/ReleaseNotes.rst clang/include/clang/AST/FormatS

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc updated this revision to Diff 455645. inclyc added a comment. Remove wchar tests. These tests may need to be re-added after we have implemented `wchar_t` checks in C Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132568/new/ https://reviews

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc added inline comments. Comment at: clang/lib/AST/FormatString.cpp:401 + if (const auto *BT = argTy->getAs()) { +if (!Ptr) { + switch (BT->getKind()) { nickdesaulniers wrote: > aaron.ballman wrote: > > It's a bit strange that we have t

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc updated this revision to Diff 455626. inclyc added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132568/new/ https://reviews.llvm.org/D132568 Files: clang/docs/ReleaseNotes.rst clang/include/clang/AST/FormatS

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc marked 2 inline comments as done. inclyc added inline comments. Comment at: clang/lib/AST/FormatString.cpp:367 +case BuiltinType::Char_U: +case BuiltinType::Bool: + return Match; aaron.ballman wrote: > Isn't this a match promotion

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc added inline comments. Comment at: clang/lib/AST/FormatString.cpp:422 + case BuiltinType::Bool: +// Don't warn printf("%hd", [char]) +// https://reviews.llvm.org/D66186 aaron.ballman wrote: > The comment is talking about pa

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc added inline comments. Comment at: clang/test/FixIt/format.m:40 -void test_object_correction (id x) { +void test_object_correction (id x) { NSLog(@"%d", x); // expected-warning{{format specifies type 'int' but the argument has type 'id'}} aaron.bal

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc updated this revision to Diff 455485. inclyc added a comment. Fix CI issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132568/new/ https://reviews.llvm.org/D132568 Files: clang/docs/ReleaseNotes.rst clang/include/clang/AST/FormatStrin

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc updated this revision to Diff 455477. inclyc added a comment. comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132568/new/ https://reviews.llvm.org/D132568 Files: clang/docs/ReleaseNotes.rst clang/include/clang/AST/FormatString.h

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc updated this revision to Diff 455475. inclyc added a comment. Comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132568/new/ https://reviews.llvm.org/D132568 Files: clang/docs/ReleaseNotes.rst clang/include/clang/AST/FormatString.h

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc updated this revision to Diff 455474. inclyc added a comment. Add tests for character literals I've noticed that Linus mentioned the following code triggered warning by clang. (With a little modifications shown below) Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.g

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc updated this revision to Diff 455459. inclyc added a comment. rm {} Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132568/new/ https://reviews.llvm.org/D132568 Files: clang/docs/ReleaseNotes.rst clang/include/clang/AST/FormatString.h c

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc updated this revision to Diff 455458. inclyc added a comment. Address comments & more tests & docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132568/new/ https://reviews.llvm.org/D132568 Files: clang/docs/ReleaseNotes.rst clang/inclu

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc added a comment. > Do we want to encode that in `test_promotion` in > `clang/test/Sema/format-strings.c`? Seems like tests on shorts are missing. Tests for short and char "incompatibility" could be found elsewhere in this file. format-strings.c void should_understand_small_integers(v

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

inclyc created this revision. inclyc added a reviewer: aaron.ballman. Herald added a subscriber: emaste. Herald added a project: All. inclyc published this revision for review. inclyc added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. https://reviews.llvm.org/D

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

inclyc added a comment. > format specifies type 'short' but the argument has type 'double' (promoted > from 'float') I'm not sure about this. I'm curious about we just consider any integer with width less than or equal to `int` may be specified by `%hhd` ? (Because of default argument promotio

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

inclyc added a comment. If some one use `%hhd` with an unmatched type argument. Should we emit diagnose like format specifies type 'int' but the argument has type 'WhateverType' instead of format specifies type 'char' but the argument has type 'WhateverType' ? It's seems that there are m

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

inclyc added a comment. In D132266#3739618 , @aaron.ballman wrote: > In D132266#3739600 , @inclyc wrote: > >> What I'm confusing is >> Which of the following two explanations is the exact meaning of `hhd`? >> >>

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

inclyc added a comment. What I'm confusing is Which of the following two explanations is the exact meaning of `hhd`? 1. consumes a 32-bit signed integer, then truncates it *inside* printf 2. consumes an 8-bit signed integer If it is the case 1, we should not emit this warning, but N2562 said tha

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

inclyc added a comment. N2562.pdf: > Modify 7.21.6.2p12: > ... > Unless a length modifier is specified, t~~T~~he corresponding argument shall > be a pointer to int ~~signed integer~~. Does this clarification statement mean that `hhd` should not be considered to correspond to `int`, but a `sig

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

inclyc added a comment. In D132266#3739513 , @aaron.ballman wrote: > Thanks for working on this @nickdesaulniers! I think we actually want to go a > slightly different direction than this and disable the diagnostics entirely. > Basically, we should be

[PATCH] D131892: [Sema] fix false -Wcomma being emitted from void returning functions

This revision was automatically updated to reflect the committed changes. Closed by commit rGccbc22cd8976: [Sema] fix false -Wcomma being emitted from void returning functions (authored by inclyc). Changed prior to commit: https://reviews.llvm.org/D131892?vs=452767&id=452970#toc Repository:

[PATCH] D131892: [Sema] fix false -Wcomma being emitted from void returning functions

inclyc added inline comments. Comment at: clang/docs/ReleaseNotes.rst:74 number of arguments cause an assertion fault. +- Fix `#57151 `_. + ``-Wcomma`` is emitted for void returning functions. Do we need an N

[PATCH] D131892: [Sema] fix false -Wcomma being emitted from void returning functions

inclyc updated this revision to Diff 452767. inclyc added a comment. Address comments. Thanks a lot for your suggestion, I noticed that the regression test tested both C and C++, so I split the test mentioned in the comment into two parts. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D131892: [Sema] fix false -Wcomma being emited from void returning functions

inclyc updated this revision to Diff 452692. inclyc added a comment. typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131892/new/ https://reviews.llvm.org/D131892 Files: clang/lib/Sema/SemaExpr.cpp clang/test/SemaCXX/warn-comma-operator.cpp

[PATCH] D131892: [Sema] fix false -Wcomma being emited from void returning functions

inclyc updated this revision to Diff 452691. inclyc added a comment. Sync comments of function `IgnoreCommaOperand` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131892/new/ https://reviews.llvm.org/D131892 Files: clang/lib/Sema/SemaExpr.cpp c

[PATCH] D131892: [Sema] fix false -Wcomma being emited from void returning functions

inclyc created this revision. Herald added a project: All. inclyc updated this revision to Diff 452665. inclyc added a comment. inclyc added reviewers: aaron.ballman, rtrieu. inclyc published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits. comments

[PATCH] D131866: [clang] fix frontend crash in auto type templates

inclyc updated this revision to Diff 452572. inclyc added a comment. . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131866/new/ https://reviews.llvm.org/D131866 Files: clang/lib/AST/ASTContext.cpp clang/test/SemaCXX/sugared-auto.cpp Index:

[PATCH] D131866: [clang] fix frontend crash in auto type templates

inclyc updated this revision to Diff 452571. inclyc added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131866/new/ https://reviews.llvm.org/D131866 Files: clang/lib/AST/ASTContext.cpp clang/test/SemaCXX/sugared-auto.cpp In

[PATCH] D131866: [clang] fix frontend crash in auto type templates

inclyc updated this revision to Diff 452569. inclyc added a comment. update comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131866/new/ https://reviews.llvm.org/D131866 Files: clang/lib/AST/ASTContext.cpp clang/test/SemaCXX/sugared-auto

  1   2   >