[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-21 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment. In D129222#3668158 , @sammccall wrote: > I think what's going on here: outer cmake invokes this opaque external > command (which just _happens_ to be cmake --build) to produce an exe. The > outer cmake has no special knowledge

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D129222#3668073 , @mizvekov wrote: > In D129222#3667638 , @hokein wrote: > >> Yeah, the native clang-pseudo-gen tool didn't rebuild somehow even its >> source file changes, it is a

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-21 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment. In D129222#3667638 , @hokein wrote: > Yeah, the native clang-pseudo-gen tool didn't rebuild somehow even its > source file changes, it is a bug in our cmake config. Should be fixed in > 2955192df8ac270515b5fa4aaa9e9380148e7f00

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-21 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. I'm sorry for the trouble it causes, and thanks for all details! > This could have something to do with "LLVM_OPTIMIZED_TABLEGEN": "ON", > perhaps, and there is some dependency missing to rebuild the NATIVE target. Yeah, the native clang-pseudo-gen tool didn't rebuild s

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks for digging! Agreed then that reverting this patch won't really solve anything. I expect I can reproduce on linux by enabling LLVM_OPTIMIZED_TABLEGEN in the same way, looking into it... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-20 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment. Yeah, turning off `LLVM_OPTIMIZED_TABLEGEN` made cmake instantly generate the correct CXXSymbols.inc. So while I think there is a bug out there somewhere, it might not necessarily be in this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-20 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment. The clang-pseudo-gen that sits in `.\llvm\RelWithDebInfo\bin\clang-pseudo-gen` generates correct output, the one that sits in `.\llvm\NATIVE\Release\bin\clang-pseudo-gen.exe` doesn't, which seems to be the one cmake is picking up for this job. This could have somethin

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-20 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment. Running `clang-pseudo-gen --grammar ..\clang-tools-extra\pseudo\lib\cxx\cxx.bnf --emit-symbol-list` does seem to generate something that would work. But even if I delete `CXXSymbols.inc`, the cmake rule seems to be regenerating a wrong CXXSymbols.inc again, so I suspec

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-20 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment. I un-reverted this locally to take a better look. It does not seem to me to be a compiler bug, because both cl.exe and intellisense are picking this error up, and I believe the MSVC intellisense is based on a completely different frontend (EDG). What I see is that `en

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-20 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment. `Microsoft (R) C/C++ Optimizing Compiler Version 19.33.31627.1 for x64` As included in `Microsoft Visual Studio 2022` `Version 17.3.0 Preview 4.0` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129222/new/ https://reviews.

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D129222#3666043 , @mizvekov wrote: > I confirm reverting just this patch fixes the build for me. Hi @mizvekov, Can you provide details about the compiler version? Neither the presubmit nor postsubmit bots (e.g.

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-20 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment. I confirm reverting just this patch fixes the build for me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129222/new/ https://reviews.llvm.org/D129222 ___ cfe-commits mailing li

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-20 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment. It seems this change broke build on windows for me: clang-tools-extra\pseudo\lib\cxx\CXX.cpp(59): error C2838: 'noptr_declarator_0declarator_id': illegal qualified name in member declaration clang-tools-extra\pseudo\lib\cxx\CXX.cpp(59): error C2065: 'noptr_declarat

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-19 Thread Haojian Wu via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd489b3807f09: [pseudo] Implement a guard to determine function declarator. (authored by hokein). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-19 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 445714. hokein marked 3 inline comments as done. hokein added a comment. restore the previous and simplified function-declarator test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129222/new/ https://reviews.ll

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. impl looks good! Comment at: clang-tools-extra/pseudo/test/cxx/declarator-function.cpp:6 // RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s -vo

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. All comments should be addressed now. As discussed offline, I'm going to land it now, happy to address any post comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129222/new/ https://reviews.llvm.org/D129222 _

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 445427. hokein marked 3 inline comments as done. hokein added a comment. rebase and add comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129222/new/ https://reviews.llvm.org/D129222 Files: clang-tools-

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-15 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 444976. hokein added a comment. rebase and address the main comment -- encoding the function-declarator into the grammar. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129222/new/ https://reviews.llvm.org/D1292

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-08 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang-tools-extra/pseudo/gen/Main.cpp:114 + const clang::pseudo::Rule &R = G.table().Rules[RID]; + // lhs$$rhs$rhs$rhs + std::string EnumName = symbolName(R.Target, G) + "$"; sammccall wrote: > hokein wrote

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-07 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/pseudo/gen/Main.cpp:114 + const clang::pseudo::Rule &R = G.table().Rules[RID]; + // lhs$$rhs$rhs$rhs + std::string EnumName = symbolName(R.Target, G) + "$"; hokein wrote: > sammccall wr

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-07 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang-tools-extra/pseudo/gen/Main.cpp:114 + const clang::pseudo::Rule &R = G.table().Rules[RID]; + // lhs$$rhs$rhs$rhs + std::string EnumName = symbolName(R.Target, G) + "$"; sammccall wrote: > so the dolla

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-07 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/pseudo/gen/Main.cpp:114 + const clang::pseudo::Rule &R = G.table().Rules[RID]; + // lhs$$rhs$rhs$rhs + std::string EnumName = symbolName(R.Target, G) + "$"; so the dollar signs are a pr

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-07 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/pseudo/gen/Main.cpp:60 + +std::string symbolName(clang::pseudo::SymbolID SID, + const clang::pseudo::Grammar &G) { This code is copied right? Comment at: clang

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-06 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Herald added a project: All. hokein requested review of this revision. Herald added a subscriber: alextsao1999. Herald added a project: clang-tools-extra. - emit an enum type to identify each grammar rule (in lhs$$rhs$rhs$ form); -