fixed #12715 - made Visual Studio conditions work again #6389
Conversation
test/testtokenlist.cpp
Outdated
| TokenList tokenlist(&settings); | ||
| std::istringstream istr(s); | ||
| tokenlist.createTokens(istr, Standards::Language::C); | ||
| tokenlist.createAst(); // TODO: do not crash |
There was a problem hiding this comment.
@chrchr-github Any idea why this might crash? It works when done in the context of Tokenizer::simplifyTokens1(). IMO the AST generation in TokenList should not depend on the simplifications.
There was a problem hiding this comment.
Looks like the parentheses are not linked (Tokenizer::createLinks()).
There was a problem hiding this comment.
The logic seems to exists in several others places:
TokenList::insertTokens()
createTokenFromExpression()
clangimport::AstNode::addTypeTokens()
I assume this is done to work around the issue I am currently encountering as the code in question is only using a TokenList.
|
This still needs Visual Studio import tests but I might not get to those. And as this is a high severity regression I want to get in a fix ASAP (as well as a backport and a 2.14.x patch release). |
|
Is the test snippet from a project file? Why do we need to create an AST from that? |
Yes. That is a bog standard condition used in every Visual Studio project file.
I need to create the AST so |
|
But a .vcxproj is XML, do we treat that as code somehow? Seems sketchy... |
This is a string extracted from it (macros obviously being replaced): <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"/>I think this is actually C# code.
Hence the TODOs to improve the testing and maybe handling it differently. |
firewave
changed the title
|
👍 |
|
This needs to be cleaned up but it is all I can do right now and this needs to be fixed ASAP since it breaks scanning Visual Studio projects. |
|
There is a WIP selftest/dogfooding job which would have caught this with #5477. |
No description provided.