[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

https://github.com/benshi001 closed https://github.com/llvm/llvm-project/pull/85921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

MaskRay wrote: We should remove the comment. To make the PR more useful, the PR can be changed to remove other confusing comments like this. https://github.com/llvm/llvm-project/pull/85921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

rjmccall wrote: Yeah, I don't think we need this change. The comment is a little unnecessary, but it's not a big deal. Really, the code ought to be using a visitor instead of a `switch`. https://github.com/llvm/llvm-project/pull/85921 ___

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

AaronBallman wrote: > So in this case, I suppose, I’ll leave it to someone more familiar w/ codegen > than me to approve/reject this pr for good. I think in this case we should remove the comment, but maybe @rjmccall or @efriedma-quic feel differently? We could also update the comment to say

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

Sirraide wrote: > Yeah, I'm also not opposed to this (an attribute is better than a comment > because the attribute can be analyzed) That was my main line of reasoning here as well. > more just wondering if we want to remove the comment rather than add the > attribute. I was wondering about

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

Sirraide wrote: > I think this is a demonstration of exactly what we _don't_ want to see in the > code base. That's just purely noise IMO. Its seem pretty unnecessary, yeah. I was fine w/ the one in this pr mainly because of the comment; my thinking was that there ought to be a reason why

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

AaronBallman wrote: > E.g: > > https://github.com/llvm/llvm-project/blob/50801f1095d33e712c3a51fdeef82569bd09007f/clang/lib/Interpreter/IncrementalParser.cpp#L141-L151 I think this is a demonstration of exactly what we *don't* want to see in the code base. That's just purely noise IMO.

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

Sirraide wrote: Admittedly, it doesn’t seem to be too common, and most other occurrences of this seem to be in LLVM proper and not in Clang. https://github.com/llvm/llvm-project/pull/85921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

Sirraide wrote: > Is this addressing a diagnostic you're seeing in the wild? This shouldn't be > necessary because the `case` statements are adjacent to one another with only > comments/whitespace between them: https://godbolt.org/z/oM7x65hq9 > > (I'd like to understand the motivation better

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

Sirraide wrote: E.g: https://github.com/llvm/llvm-project/blob/50801f1095d33e712c3a51fdeef82569bd09007f/clang/lib/Interpreter/IncrementalParser.cpp#L141-L151 https://github.com/llvm/llvm-project/pull/85921 ___ cfe-commits mailing list

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

https://github.com/AaronBallman commented: Is this addressing a diagnostic you're seeing in the wild? This shouldn't be necessary because the `case` statements are adjacent to one another with only comments/whitespace between them: https://godbolt.org/z/oM7x65hq9 (I'd like to understand the

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

https://github.com/Sirraide approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/85921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

https://github.com/benshi001 updated https://github.com/llvm/llvm-project/pull/85921 >From 707adafab92900392ed5aabffa678afe9b4903d7 Mon Sep 17 00:00:00 2001 From: Ben Shi Date: Wed, 20 Mar 2024 19:36:50 +0800 Subject: [PATCH] [clang][AST][NFC] Add '[[fallthrough]];' to cases fall through ---

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff e2fa90fa0a4b7950dd0d7fae6933e89c075d0af0 144119d57d181fb16e27a5c7d869422a39185978 --

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

llvmbot wrote: @llvm/pr-subscribers-clang-codegen Author: Ben Shi (benshi001) Changes --- Full diff: https://github.com/llvm/llvm-project/pull/85921.diff 1 Files Affected: - (modified) clang/lib/CodeGen/CGStmt.cpp (+1-1) ``diff diff --git a/clang/lib/CodeGen/CGStmt.cpp

[clang] [clang][AST][NFC] Add '[[fallthrough]]' to cases fall through (PR #85921)

https://github.com/benshi001 created https://github.com/llvm/llvm-project/pull/85921 None >From 144119d57d181fb16e27a5c7d869422a39185978 Mon Sep 17 00:00:00 2001 From: Ben Shi Date: Wed, 20 Mar 2024 19:36:50 +0800 Subject: [PATCH] [clang][AST][NFC] Add '[[fallthrough]];' to cases fall