GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — made it possible to create a `TokenList` from a buffer by firewave · Pull Request #347 · cppcheck-opensource/simplecpp

Conversation

@firewave

@firewave firewave commented Mar 27, 2024

Copy link
Copy Markdown
Collaborator

No description provided.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@firewave

firewave commented Mar 27, 2024

Copy link
Copy Markdown
Collaborator Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@firewave

firewave commented Mar 28, 2024

Copy link
Copy Markdown
Collaborator Author

Some tests are still failing - will investigate.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@firewave

firewave commented Mar 28, 2024

Copy link
Copy Markdown
Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@firewave firewave changed the title added StdStringStream which reads from a std::string added constructors to create a TokenList from a abuffer Mar 29, 2024
@firewave firewave changed the title added constructors to create a TokenList from a abuffer added constructors to create a TokenList from a buffer Mar 29, 2024
@firewave
firewave force-pushed the stdstringstream branch from 71a7916 to 32f1606 Compare March 29, 2024 18:17
@firewave

firewave commented Mar 29, 2024

Copy link
Copy Markdown
Collaborator Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

firewave added 3 commits March 29, 2024 19:40
@firewave
added StdCharBufStream which reads from a buffer
1696577
@firewave
use StdCharBufStream in Macro
1cb98c8
@firewave
added TokenList constructors which take a buffer
1895980
@firewave
firewave force-pushed the stdstringstream branch from 32f1606 to 1895980 Compare March 29, 2024 18:41
@firewave

firewave commented Mar 29, 2024

Copy link
Copy Markdown
Collaborator Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@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
@firewave firewave changed the title added constructors to create a TokenList from a buffer made it possible to create a TokenList from a buffer Mar 29, 2024
@firewave
firewave marked this pull request as ready for review March 29, 2024 22:06
@firewave

firewave commented Apr 30, 2024

Copy link
Copy Markdown
Collaborator Author

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

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@danmar

danmar commented May 3, 2024

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

firewave commented May 3, 2024

Copy link
Copy Markdown
Collaborator Author

@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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

firewave
firewave commented May 3, 2024
View reviewed changes
Comment thread simplecpp.cpp
}

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

@firewave firewave May 3, 2024

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@firewave firewave mentioned this pull request May 3, 2024
pass a char buffer to simplecpp instead of a stream cppcheck-opensource/cppcheck#6379
Merged
@firewave

firewave commented May 3, 2024

Copy link
Copy Markdown
Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@firewave

firewave commented May 16, 2024

Copy link
Copy Markdown
Collaborator Author

Ready for review since the downstream PR passes all tests.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@firewave
firewave marked this pull request as ready for review May 16, 2024 13:51
danmar
danmar reviewed May 21, 2024
View reviewed changes
Comment thread simplecpp.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread simplecpp.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread simplecpp.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.

danmar
danmar approved these changes May 23, 2024
View reviewed changes

@danmar danmar left a comment

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@danmar

danmar commented May 23, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@firewave

firewave commented May 23, 2024

Copy link
Copy Markdown
Collaborator Author

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

@danmar Thanks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

Reviewers

@danmar danmar danmar approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@firewave @danmar
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.