[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added a comment. In D129048#3692394 , @thakis wrote: > This change caused a ton of churn, for what I understand fix the diag in C > mode when assert.h is not included. Which is not that uncommon of a scenario: https://sourcegraph.com/sear

[PATCH] D129048: Rewording the "static_assert" to static assertion

thakis added a comment. This change caused a ton of churn, for what I understand fix the diag in C mode when assert.h is not included. IMHO, it would've been better to change the diag to say either `static_assert` as before in c++ mode or when assert.h is included (i.e. almost always), and `_S

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added a comment. In D129048#3674585 , @Codesbyusman wrote: > In D129048#3674582 , @Mordante > wrote: > >> In D129048#3674500 , @Codesbyusman >> wrote: >>

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman added a comment. In D129048#3674582 , @Mordante wrote: > In D129048#3674500 , @Codesbyusman > wrote: > >> updated the libcxx all files > > FYI for libcxx you can ignore the red format CI step. So the

[PATCH] D129048: Rewording the "static_assert" to static assertion

Mordante accepted this revision. Mordante added a comment. This revision is now accepted and ready to land. In D129048#3674500 , @Codesbyusman wrote: > updated the libcxx all files FYI for libcxx you can ignore the red format CI step. So the libcxx buil

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman added a comment. In D129048#3671657 , @ldionne wrote: > In D129048#3671568 , @aaron.ballman > wrote: > >> FWIW, I've convinced myself that I agree with you here that the burden >> probably should ha

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman added a comment. In D129048#3671665 , @erichkeane wrote: > In D129048#3671657 , @ldionne wrote: > >> In D129048#3671568 , >> @aaron.ballman wrote: >> >>> FW

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman added a comment. In D129048#3671568 , @aaron.ballman wrote: > In D129048#3670212 , @ldionne wrote: > >> The libc++ CI runs pre-commit only, and it runs on all commits that touch >> something under `

[PATCH] D129048: Rewording the "static_assert" to static assertion

erichkeane added a comment. In D129048#3671657 , @ldionne wrote: > In D129048#3671568 , @aaron.ballman > wrote: > >> FWIW, I've convinced myself that I agree with you here that the burden >> probably should have

[PATCH] D129048: Rewording the "static_assert" to static assertion

ldionne added a comment. In D129048#3671568 , @aaron.ballman wrote: > FWIW, I've convinced myself that I agree with you here that the burden > probably should have been on libc++ maintainers in this case. libc++ almost > feels more like a downstream co

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added a comment. In D129048#3670212 , @ldionne wrote: > The libc++ CI runs pre-commit only, and it runs on all commits that touch > something under `libcxx/`, `libcxxabi/`, `runtimes/` and `libunwind/`. In > particular, it won't trigger if

[PATCH] D129048: Rewording the "static_assert" to static assertion

ldionne added a comment. The libc++ CI runs pre-commit only, and it runs on all commits that touch something under `libcxx/`, `libcxxabi/`, `runtimes/` and `libunwind/`. In particular, it won't trigger if you make changes to `clang/` only -- that would create too much traffic and concretely it

[PATCH] D129048: Rewording the "static_assert" to static assertion

Mordante added a comment. In D129048#3669568 , @aaron.ballman wrote: > In D129048#3669069 , @Mordante > wrote: > >> In D129048#3668905 , >> @aaron.ballman wrote: >> >>>

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added a comment. In D129048#3669069 , @Mordante wrote: > In D129048#3668905 , @aaron.ballman > wrote: > >> In D129048#3668846 , @ldionne >> wrote: >> >>> I

[PATCH] D129048: Rewording the "static_assert" to static assertion

Mordante added a comment. In D129048#3668905 , @aaron.ballman wrote: > In D129048#3668846 , @ldionne wrote: > >> In D129048#3668594 , @philnik >> wrote: >> >>> Also, ple

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added a comment. In D129048#3668846 , @ldionne wrote: > In D129048#3668594 , @philnik wrote: > >> Also, please wait for #libc approval >> next time. > > This, x1

[PATCH] D129048: Rewording the "static_assert" to static assertion

ldionne added a comment. In D129048#3668594 , @philnik wrote: > Also, please wait for #libc approval > next time. This, x1000. We go through the trouble of having excellent pre-commit testing and automatic review g

[PATCH] D129048: Rewording the "static_assert" to static assertion

philnik added a comment. Also, please wait for #libc approval next time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 _

[PATCH] D129048: Rewording the "static_assert" to static assertion

philnik added a comment. In D129048#3668542 , @erichkeane wrote: > Tests ran clean for me as well. Looking at the CI, it appears that the > libcxx tests are improperly configured, and are being run against an 'older' > version of the compiler (not inc

[PATCH] D129048: Rewording the "static_assert" to static assertion

erichkeane added a comment. Tests ran clean for me as well. Looking at the CI, it appears that the libcxx tests are improperly configured, and are being run against an 'older' version of the compiler (not including this patch). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman added a comment. In D129048#3662972 , @xgupta wrote: > In D129048#3662634 , @Codesbyusman > wrote: > >> the libcxx updates but not tested. working on them > > I just run the check-cxx on local system

[PATCH] D129048: Rewording the "static_assert" to static assertion

xgupta added a comment. In D129048#3662634 , @Codesbyusman wrote: > the libcxx updates but not tested. working on them I just run the check-cxx on local system. It seems to pass all testcases after applying the current patch. Testing Time: 2147.75s

[PATCH] D129048: Rewording the "static_assert" to static assertion

ldionne added a comment. Most of the libc++ CI testing happens on BuildKite, and we only trigger the libc++ buildkite pipeline if files in `libcxx/` (and a few others) have changed. This wasn't the case here. If you want to re-land without breaking libc++, you'd have to go through libc++'s `.ve

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added a comment. In D129048#3652739 , @hctim wrote: > In D129048#3652689 , @aaron.ballman > wrote: > >> In D129048#3652679 , @hctim wrote: >> >>> Looks like

[PATCH] D129048: Rewording the "static_assert" to static assertion

hctim added a comment. In D129048#3652689 , @aaron.ballman wrote: > In D129048#3652679 , @hctim wrote: > >> Looks like unfortunately this breaks all lots of libcxx tests - which were >> picked up by our sanitize

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added a comment. In D129048#3652679 , @hctim wrote: > Looks like unfortunately this breaks all lots of libcxx tests - which were > picked up by our sanitizer buildbots: Thanks for letting us know. Do you happen to know if there's any part

[PATCH] D129048: Rewording the "static_assert" to static assertion

hctim added a comment. Looks like unfortunately this breaks all lots of libcxx tests - which were picked up by our sanitizer buildbots: https://lab.llvm.org/buildbot/#/builders/85/builds/9157 Should be reproducible with just `cmake -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi" && make check-cxx`.

[PATCH] D129048: Rewording the "static_assert" to static assertion

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb7e77ff25fb2: Rewording "static_assert" diagnostics (authored by Codesbyusman, committed by aaron.ballman). Changed prior to commit: https://revie

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman added a comment. ok. I will take care of this next time. Thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 ___ cfe-commits mailing list cf

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. The changes here LGTM aside from one thing (which I'll take care of when I land the changes on your behalf). ParseDeclCXX.cpp still has a bunch of unrelated whitespace changes in

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman added a comment. In D129048#3648964 , @erichkeane wrote: > Please format your patch with clang-format. See the command at the end of > this: https://clang.llvm.org/docs/ClangFormat.html > > `git diff -U0 --color=never | clang-format-diff.py

[PATCH] D129048: Rewording the "static_assert" to static assertion

erichkeane added a comment. Please format your patch with clang-format. See the command at the end of this: https://clang.llvm.org/docs/ClangFormat.html `git diff -U0 --color=never | clang-format-diff.py -i -p1` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

Re: [PATCH] D129048: Rewording the "static_assert" to static assertion

Done. On Wed, Jul 13, 2022 at 10:30 PM Aaron Ballman via Phabricator < revi...@reviews.llvm.org> wrote: > aaron.ballman added inline comments. > > > > Comment at: > clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp:48 > } > \ No newline at end of file > > --

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added inline comments. Comment at: clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp:48 } \ No newline at end of file Codesbyusman wrote: > aaron.ballman wrote: > > xgupta wrote: > > > a 'newline' is missing here. > > Still missi

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman marked an inline comment as done. Codesbyusman added inline comments. Comment at: clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp:48 } \ No newline at end of file aaron.ballman wrote: > xgupta wrote: > > a 'newline' is missing h

[PATCH] D129048: Rewording the "static_assert" to static assertion

erichkeane added inline comments. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:995 DeclEnd = Tok.getLocation(); - ExpectAndConsumeSemi(diag::err_expected_semi_after_static_assert); + ExpectAndConsumeSemi(diag::err_expected_semi_after_static_assert , SavedTok.getName()); //p

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:929 + // saving the token used for static assertion. + Token SavedTok = Tok; Comment at: clang/lib/Parse/ParseDeclCXX.cpp:995 DeclEnd = Tok.getLocation();

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman updated this revision to Diff 444311. Codesbyusman added a comment. updated the comment styles suggested Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 Files: clang/include/clang/Basic/Dia

[PATCH] D129048: Rewording the "static_assert" to static assertion

erichkeane added a comment. A couple of comment suggestions, our comments use a full-stop. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:929 + // saving the token used for static assertion + Token SavedTok = Tok; Comment at: clang/lib/Par

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman updated this revision to Diff 444284. Codesbyusman added a comment. the tests for the coverage of changes being made for the static assertion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 F

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman marked 3 inline comments as done. Codesbyusman added a comment. In D129048#3647918 , @aaron.ballman wrote: > I think this is getting pretty close! I did have some style guideline > changes, and at least one test addition I'd like to see tho

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman updated this revision to Diff 444250. Codesbyusman added a comment. updated the styles and other small things Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 Files: clang/include/clang/Basi

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added a comment. I think this is getting pretty close! I did have some style guideline changes, and at least one test addition I'd like to see though. Also, it looks like precommit CI caught a test that still needs to be updated: FAIL: Clang :: SemaCXX/cxx98-compat.cpp (16448 of

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman updated this revision to Diff 444211. Codesbyusman added a comment. the parser updates maked for the static asserion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 Files: clang/include/cla

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added a comment. In D129048#3630584 , @Codesbyusman wrote: > Yes I am looking into the DiagnosticParseKinds.td Excellent, thank you! In D129048#3630589 , @Codesbyusman wrote: > Yes I am looking

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman added a comment. Yes I am looking into the DiagnosticParseKinds.td. Kindly also look another error encountered in testing while updating the test. Although not having in my system but this is giving . Look here

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman added a comment. Yes I am looking into the DiagnosticParseKinds.td Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 ___ cfe-commits mailing list cfe

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added a comment. Precommit CI failures currently look to be unrelated. I think the only thing remaining is this bit: > I think we should also update DiagnosticParseKinds.td at the same time... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman updated this revision to Diff 442299. Codesbyusman added a comment. updating the test files for the last update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 Files: clang/include/clang/Ba

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman updated this revision to Diff 442276. Codesbyusman added a comment. updating some to the back strings becaue the orginal were more better Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 Files

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added a comment. Thank you for getting a great start on this diagnostic rewording! I think we should also update DiagnosticParseKinds.td at the same time, but because that's about parsing rather than semantics, not all of the diagnostics should be reworded to use "static assertion

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman updated this revision to Diff 442199. Codesbyusman added a comment. new changes as required -- final Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 Files: clang/include/clang/Basic/Diagnos

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman updated this revision to Diff 442197. Codesbyusman added a comment. update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/te

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman updated this revision to Diff 442191. Codesbyusman added a comment. all Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/test/

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman updated this revision to Diff 442189. Codesbyusman added a comment. all changes in one patch revision Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 Files: clang/include/clang/Basic/Diagno

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman updated this revision to Diff 442188. Codesbyusman added a comment. All changes in one place Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 Files: clang/include/clang/Basic/DiagnosticSemaK

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman updated this revision to Diff 442184. Codesbyusman added a comment. updating Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman updated this revision to Diff 442183. Codesbyusman added a comment. Updating the files as required Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129048/new/ https://reviews.llvm.org/D129048 Files: clang/include/clang/Basic/Diagnosti

[PATCH] D129048: Rewording the "static_assert" to static assertion

xgupta added a comment. Three test cases are failing - Please see: https://buildkite.com/llvm-project/premerge-checks/builds/100784#0181c539-bcd1-4d01-a2db-f27a63e2f17e/6-15379 But I think you want to actually modify https://github.com/llvm/llvm-project/blob/93d6fdfc232c59975d52146532693178def5a

[PATCH] D129048: Rewording the "static_assert" to static assertion

Codesbyusman created this revision. Codesbyusman added reviewers: aaron.ballman, erichkeane, xgupta. Herald added a project: All. Codesbyusman requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Rewording the failing of _Satic_assert to the bet