erichkeane added a comment.
In D119609#3424434 <https://reviews.llvm.org/D119609#3424434>, @junaire wrote:
> Hi @aaron.ballman, do you think it's time to consider reviewing this?
>
> I don't why there's no response or progress in GCC, but I think I can submit
> a patch for them if they think this issue is low prior.
I think I'm ok with rejecting both cases now, and doing this review. Though
I'm still concerned with the 'skip-until' losing some diagnostics, so please
add another test for that as I requested before.
================
Comment at: clang/test/Sema/err-expr-stmt-in-default-arg.cpp:3
+
+void foo() {
+ void fn(int i, int = ({ 1; })); // expected-error {{expression statement not
permitted in default argument}}
----------------
Do we also prohibit in a template argument?
================
Comment at: clang/test/Sema/err-expr-stmt-in-default-arg.cpp:19
+ return bar(l);
+}
----------------
erichkeane wrote:
> junaire wrote:
> > erichkeane wrote:
> > > For completeness, I'd like to see an
> > > in-function-defined-struct-member-function test here as well.
> > >
> > > As for the above question about the recovery, something like:
> > >
> > > `void fn(int i, int j = ({ {},{},{,} }), int k = "");` I think checks all
> > > the issues I can think of. We want to make sure that 'k' still diagnoses
> > > its error correctly (and that we have just skipped all of the expression
> > > statement stuff).
> > Note that clang is already rejected the code:
> > https://godbolt.org/z/57c4qaaPo
> >
> I more mean an example like this one:
> https://godbolt.org/z/nf7W1zznh
>
> I see that we already reject it, however you are doing a 'skip until' here,
> and I want to make sure that the error on 'k' diagnoses correctly still.
>
> With your change, I would expect the 1st two errors there to go away and be
> replaced by your new error. BUT the 'k' type would still be there.
>
> The point of this is to make sure that your error doesn't leave the parser in
> a 'bad' state.
I don't see the test I requested here, I'd still like to see that to make sure
the parser still diagnoses' k'.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119609/new/
https://reviews.llvm.org/D119609
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
