[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

2022-08-12 Thread YingChi Long via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe5825190b8ad: [clang] fix frontend crash when evaluating type trait (authored by inclyc). Changed prior to commit: https://reviews.llvm.org/D13142

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

2022-08-14 Thread YingChi Long via Phabricator via cfe-commits
inclyc created this revision. Herald added a project: All. inclyc added reviewers: mizvekov, aaron.ballman. inclyc updated this revision to Diff 452566. inclyc added a comment. inclyc added subscribers: cfe-commits, clang. inclyc published this revision for review. Herald added a project: clang. i

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

2022-08-14 Thread YingChi Long via Phabricator via cfe-commits
inclyc updated this revision to Diff 452567. 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

2022-08-14 Thread YingChi Long via Phabricator via cfe-commits
inclyc updated this revision to Diff 452568. inclyc added a comment. rename 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

2022-08-14 Thread YingChi Long via Phabricator via cfe-commits
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

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

2022-08-14 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-14 Thread YingChi Long via Phabricator via cfe-commits
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] D131892: [Sema] fix false -Wcomma being emited from void returning functions

2022-08-15 Thread YingChi Long via Phabricator via cfe-commits
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] D131892: [Sema] fix false -Wcomma being emited from void returning functions

2022-08-15 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-15 Thread YingChi Long via Phabricator via cfe-commits
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 emitted from void returning functions

2022-08-15 Thread YingChi Long via Phabricator via cfe-commits
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 emitted from void returning functions

2022-08-15 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-16 Thread YingChi Long via Phabricator via cfe-commits
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] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

2022-08-22 Thread YingChi Long via Phabricator via cfe-commits
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] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

2022-08-22 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-22 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-22 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-22 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-22 Thread YingChi Long via Phabricator via cfe-commits
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] D132568: [clang][Sema] check default argument promotions for printf

2022-08-24 Thread YingChi Long via Phabricator via cfe-commits
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] D132568: [clang][Sema] check default argument promotions for printf

2022-08-24 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-24 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-24 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-24 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-24 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-24 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-25 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-25 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-25 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-25 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-25 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-25 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-25 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-25 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-25 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-25 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-26 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-26 Thread YingChi Long via Phabricator via cfe-commits
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

2022-08-26 Thread YingChi Long via Phabricator via cfe-commits
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] D133574: [C2x] reject type definitions in offsetof

2022-09-15 Thread YingChi Long via Phabricator via cfe-commits
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

2022-09-19 Thread YingChi Long via Phabricator via cfe-commits
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

2022-09-21 Thread YingChi Long via Phabricator via cfe-commits
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

2022-09-23 Thread YingChi Long via Phabricator via cfe-commits
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

2022-09-23 Thread YingChi Long via Phabricator via cfe-commits
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

2022-09-24 Thread YingChi Long via Phabricator via cfe-commits
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

2022-09-26 Thread YingChi Long via Phabricator via cfe-commits
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

2022-09-26 Thread YingChi Long via Phabricator via cfe-commits
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

2022-09-26 Thread YingChi Long via Phabricator via cfe-commits
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

2022-09-26 Thread YingChi Long via Phabricator via cfe-commits
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

2022-09-26 Thread YingChi Long via Phabricator via cfe-commits
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

2022-09-26 Thread YingChi Long via Phabricator via cfe-commits
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] D134815: [Sema] print more readable identifier of anonymous struct of -Wconsumed

2022-09-28 Thread YingChi Long via Phabricator via cfe-commits
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] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

2022-10-10 Thread YingChi Long via Phabricator via cfe-commits
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] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

2022-10-10 Thread YingChi Long via Phabricator via cfe-commits
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

2022-10-10 Thread YingChi Long via Phabricator via cfe-commits
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

2022-10-10 Thread YingChi Long via Phabricator via cfe-commits
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

2022-10-11 Thread YingChi Long via Phabricator via cfe-commits
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

2022-10-11 Thread YingChi Long via Phabricator via cfe-commits
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

2022-10-11 Thread YingChi Long via Phabricator via cfe-commits
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] D133574: [C2x] reject type definitions in offsetof

2023-01-01 Thread Yingchi Long via Phabricator via cfe-commits
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] D137839: [Sema] check InitListExpr format strings like {"foo"}

2022-11-11 Thread Yingchi Long via Phabricator via cfe-commits
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] D137839: [Sema] check InitListExpr format strings like {"foo"}

2022-11-17 Thread Yingchi Long via Phabricator via cfe-commits
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] D133574: [C2x] reject type definitions in offsetof

2023-01-12 Thread Yingchi Long via Phabricator via cfe-commits
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

2023-01-12 Thread Yingchi Long via Phabricator via cfe-commits
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

2023-01-16 Thread Yingchi Long via Phabricator via cfe-commits
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] D137839: [Sema] check InitListExpr format strings like {"foo"}

2022-11-22 Thread Yingchi Long via Phabricator via cfe-commits
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"}

2022-11-22 Thread Yingchi Long via Phabricator via cfe-commits
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"}

2022-11-22 Thread Yingchi Long via Phabricator via cfe-commits
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] D137343: [clang] add -Wvla-stack-allocation

2022-11-22 Thread Yingchi Long via Phabricator via cfe-commits
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] D132952: [Sema] disable -Wvla for function array parameters

2022-11-24 Thread Yingchi Long via Phabricator via cfe-commits
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] D143439: [RISCV] Add vendor-defined XTheadBb (basic bit-manipulation) extension

2023-02-13 Thread Yingchi Long via Phabricator via cfe-commits
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] D143439: [RISCV] Add vendor-defined XTheadBb (basic bit-manipulation) extension

2023-02-13 Thread Yingchi Long via Phabricator via cfe-commits
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] D150001: [clang] Fix initializer_list matching failures with modules

2023-05-06 Thread Yingchi Long via Phabricator via cfe-commits
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] D137343: [clang] add -Wvla-stack-allocation

2022-12-07 Thread Yingchi Long via Phabricator via cfe-commits
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] D133574: [C2x] reject type definitions in offsetof

2022-12-13 Thread Yingchi Long via Phabricator via cfe-commits
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] D133574: [C2x] reject type definitions in offsetof

2022-12-13 Thread Yingchi Long via Phabricator via cfe-commits
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

<    1   2