[clang] [clang] fix the unexpected control flow in ParseTentative.cpp (PR #109298)

2024-09-27 Thread via cfe-commits
c8ef wrote: I'm sorry, but I can't identify a case that would trigger this code path. I might not be that familiar with ObjC... https://github.com/llvm/llvm-project/pull/109298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[clang] [clang] fix the unexpected control flow in ParseTentative.cpp (PR #109298)

2024-09-21 Thread via cfe-commits
cor3ntin wrote: > @cor3ntin See the discussion on #95917. This is likely effectively dead code, > but nobody is certain enough of that to delete it outright... The original issue seems to suggest the code path can be used on invalid code such as struct X { }; void foo() { X; } ``` (ie

[clang] [clang] fix the unexpected control flow in ParseTentative.cpp (PR #109298)

2024-09-20 Thread via cfe-commits
c8ef wrote: > But that function _does_ have a return statement after the loop? I assumed that the entire function body was enclosed within the `while(true)` loop? https://github.com/llvm/llvm-project/pull/109298 ___ cfe-commits mailing list cfe-commi

[clang] [clang] fix the unexpected control flow in ParseTentative.cpp (PR #109298)

2024-09-20 Thread via cfe-commits
c8ef wrote: Or we can simply return `TPResult::Ambiguous`, as many functions in this file do. https://github.com/llvm/llvm-project/pull/109298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[clang] [clang] fix the unexpected control flow in ParseTentative.cpp (PR #109298)

2024-09-20 Thread Timm Baeder via cfe-commits
tbaederr wrote: But that function _does_ have a return statement after the loop? https://github.com/llvm/llvm-project/pull/109298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] fix the unexpected control flow in ParseTentative.cpp (PR #109298)

2024-09-20 Thread via cfe-commits
c8ef wrote: > I'm only worried about the missing return at the end of the function - maybe > there should be a `llvm_unreachable` here to silence compilers, even if that > control flow is not possible. https://github.com/llvm/llvm-project/blob/21594f2793da5d2e1d1cd6714bfa10e742f2e526/clang/lib

[clang] [clang] fix the unexpected control flow in ParseTentative.cpp (PR #109298)

2024-09-20 Thread Timm Baeder via cfe-commits
tbaederr wrote: Yeah the behavior is not the same. The original code only parsed `ident[,]` in the loop, since the `continue` ended the loop anyway. The comment above the function reads: ``` /// [ObjC] protocol-qualifiers: '<' identifier-list '>' ``` So the assumption is that this

[clang] [clang] fix the unexpected control flow in ParseTentative.cpp (PR #109298)

2024-09-20 Thread Shafik Yaghmour via cfe-commits
shafik wrote: CC @tbaederr this does not look like equivalent code to me based on your original isue but maybe I am confused. https://github.com/llvm/llvm-project/pull/109298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[clang] [clang] fix the unexpected control flow in ParseTentative.cpp (PR #109298)

2024-09-19 Thread Nikita Popov via cfe-commits
nikic wrote: @cor3ntin See the discussion on https://github.com/llvm/llvm-project/pull/95917. This is very likely effectively dead code, but nobody is certain enough of that to delete it outright... https://github.com/llvm/llvm-project/pull/109298 _

[clang] [clang] fix the unexpected control flow in ParseTentative.cpp (PR #109298)

2024-09-19 Thread via cfe-commits
c8ef wrote: Based on the comment from the original PR, I am unsure how to reproduce this bug. However, it aligns with the original intention of this code. https://github.com/llvm/llvm-project/pull/109298 ___ cfe-commits mailing list cfe-commits@lists.

[clang] [clang] fix the unexpected control flow in ParseTentative.cpp (PR #109298)

2024-09-19 Thread via cfe-commits
cor3ntin wrote: Do we have a test for this? https://github.com/llvm/llvm-project/pull/109298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] fix the unexpected control flow in ParseTentative.cpp (PR #109298)

2024-09-19 Thread via cfe-commits
llvmbot wrote: @llvm/pr-subscribers-clang Author: None (c8ef) Changes Fixes https://github.com/llvm/llvm-project/issues/95895. It appears that there was a logic error in the code of Parser::TryParseProtocolQualifiers related to parsing the identifier list. This pull request fixed the is

[clang] [clang] fix the unexpected control flow in ParseTentative.cpp (PR #109298)

2024-09-19 Thread via cfe-commits
https://github.com/c8ef created https://github.com/llvm/llvm-project/pull/109298 Fixes https://github.com/llvm/llvm-project/issues/95895. It appears that there was a logic error in the code of Parser::TryParseProtocolQualifiers related to parsing the identifier list. This pull request fixed th