Skip to content

made it possible to create a TokenList from a buffer - #347

Merged
firewave merged 3 commits into
cppcheck-opensource:masterfrom
firewave:stdstringstream
May 24, 2024
Merged

made it possible to create a TokenList from a buffer#347
firewave merged 3 commits into
cppcheck-opensource:masterfrom
firewave:stdstringstream

Conversation

@firewave

Copy link
Copy Markdown
Collaborator

No description provided.

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

Still need to integrate/test it with #261 to make sure it does not introduce any issues.

It is already actively used by Macro though and no tests are failing, so it should be fine.

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

Some tests are still failing - will investigate.

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

The tests failing are the ones with binary characters in the string. We need to add an additional const char*, int input for those.

@firewavefirewave changed the title added StdStringStream which reads from a std::stringadded constructors to create a TokenList from a abufferMar 29, 2024
@firewavefirewave changed the title added constructors to create a TokenList from a abufferadded constructors to create a TokenList from a bufferMar 29, 2024
@firewave

Copy link
Copy Markdown
CollaboratorAuthor

I changed it from a std::string input to a const unsigned char*, std::size_t and const char*, std::size_t (for convienience).

The tests pass but it produces valgrind failures in #261.

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

The tests pass but it produces valgrind failures in #261.

The issue has been fixed - see https://github.com/danmar/simplecpp/actions/runs/8484373804/job/23247192053?pr=261.

@firewave
firewave marked this pull request as ready for review March 29, 2024 18:43
@firewave
firewave marked this pull request as draft March 29, 2024 18:43
@firewavefirewave changed the title added constructors to create a TokenList from a buffermade it possible to create a TokenList from a bufferMar 29, 2024
@firewave
firewave marked this pull request as ready for review March 29, 2024 22:06
@firewave

Copy link
Copy Markdown
CollaboratorAuthor

I will provide a downstream PR which shows this is working within Cppcheck.

We should also do the (planned) release before merging this.

@danmar

Copy link
Copy Markdown
Collaborator

@firewave would you like that I release simplecpp now or do you have some more fixes..?

@firewave
firewave marked this pull request as draft May 3, 2024 13:12
@firewave

Copy link
Copy Markdown
CollaboratorAuthor

@firewave would you like that I release simplecpp now or do you have some more fixes..?

Do the release so we are in proper sync again.

Afterwards this PR, #339 and #312 require your feedback - and some minor work from my side.

Comment threadsimplecpp.cpp
}

virtual int get() OVERRIDE {
if (pos >= size)

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These checks are still a hot spot. No idea if there is a way to implement this in a more performant way.

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

Here is the downstream PR using char buffers instead of streams: cppcheck-opensource/cppcheck#6379.

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

Ready for review since the downstream PR passes all tests.

@firewave
firewave marked this pull request as ready for review May 16, 2024 13:51
Comment threadsimplecpp.cpp
Comment threadsimplecpp.cpp
Comment threadsimplecpp.cpp

@danmardanmar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@danmar

danmar commented May 23, 2024

Copy link
Copy Markdown
Collaborator

@firewave you should have gotten an invite for this project. I hope you can merge..

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

you should have gotten an invite for this project. I hope you can merge..

@danmar Thanks.

@firewave
firewave merged commit d904016 into cppcheck-opensource:masterMay 24, 2024
@firewave
firewave deleted the stdstringstream branch May 24, 2024 06:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@firewave@danmar