[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D76696#1944825 , @hubert.reinterpretcast wrote: > In D76696#1944784 , @sammccall wrote: > > > The general scheme is probably common: unresolved expr -> ??? -> an > > expression is dep

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. In D76696#1944784 , @sammccall wrote: > The general scheme is probably common: unresolved expr -> ??? -> an > expression is dependent but not marked as such -> constant evaluation crashes. > > But the ??? matters, a

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D76696#1944625 , @hubert.reinterpretcast wrote: > All of them appear to have unresolved names in constexpr evaluation. It is > likely to be the same issue. The general scheme is probably common: unresolved expr -> ??? -> a

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. > In D76696#1944014 , > @hubert.reinterpretcast wrote: > >> We have also encountered crashes in downstream testing caused by this using >> just the vanilla source from trunk. When there is a proposed fix, please le

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. @ebevhan Thanks again for the testcase. I've added a reduced version in 47e7bdb10732e6f140adce39a1bc34e3ee2a6ea6 to ensure this is fixed on re-land. This particular case can be fixed by marking the

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. We have also encountered crashes in downstream testing caused by this using just the vanilla source from trunk. When there is a proposed fix, please let us know so we can test. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. Thanks for the test case. Reverted in https://github.com/llvm/llvm-project/commit/62dea6e9be31b100962f9ad41c1b79467a53f6cd for now. Adding the error-bit to type looks like a right direction. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D76696#1943657 , @ebevhan wrote: > However, this just means that `baz` has a very odd type; it's a variable of > closure type that contains a dependent member that will never be resolved. > Our diagnostic breaks, since the v

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added a comment. We found an odd case in our downstream fork after this patch landed. We have a diagnostic in `CheckVariableDeclarationType` that checks for automatic variables that are too large for the stack address space, and it chokes on the testcase `Parser/objcxx11-invalid-lambda.

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-25 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG0788acbccbec: [AST] Build recovery expressions by default for C++. (authored by hokein). Changed prior to commit: https://reviews.llvm.org/D76696?vs=252513&id=252515#toc Repository: rG LLVM Github Mo

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In D76696#1939513 , @sammccall wrote: > Do you also want to update LangOpts.td to make the default for the langopt > equal to CPlusPlus? > (I saw other opts doing that, not sure exactly what it affects, may be > voodoo cargo cult

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 252513. hokein marked an inline comment as done. hokein added a comment. address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76696/new/ https://reviews.llvm.org/D76696 Files: clang-tools-ex

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Do you also want to update LangOpts.td to make the default for the langopt equal to CPlusPlus? (I saw other opts doing that, not sure exactly what it affects, may be voodoo cargo cult stuff) Comment at: clang-tools-extra/clangd/unittests/CodeComplet

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-24 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 252320. hokein added a comment. Herald added subscribers: usaxena95, kadircet, arphaman, jkorous. fix the clangd test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76696/new/ https://reviews.llvm.org/D76696 Fi

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-24 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D76696 Files: clang/lib/Frontend/CompilerInvocation.cpp clang/test/OpenMP/target_update_from_messages.cpp clang/test/OpenMP/target