Skip to content

fix truncation of token flags#4727

Merged
danmar merged 1 commit intocppcheck-opensource:mainfrom
IOBYTE:main
Jan 26, 2023
Merged

fix truncation of token flags#4727
danmar merged 1 commit intocppcheck-opensource:mainfrom
IOBYTE:main

Conversation

@IOBYTE
Copy link
Copy Markdown
Contributor

@IOBYTE IOBYTE commented Jan 18, 2023

No description provided.

@firewave
Copy link
Copy Markdown
Collaborator

/mnt/s/GitHub/cppcheck-fw/lib/token.h:446:16: warning: implicit conversion loses integer precision: 'const uint64_t' (aka 'const unsigned long') to 'unsigned int' [-Wshorten-64-to-32]
        return mFlags;
        ~~~~~~ ^~~~~~

@chrchr-github
Copy link
Copy Markdown
Collaborator

chrchr-github commented Jan 18, 2023

-Werror?
Edit: I guess you mean there was a warning before?

@IOBYTE
Copy link
Copy Markdown
Contributor Author

IOBYTE commented Jan 18, 2023

That makes no sense. mFlags is a uint64_t. Are you sure you have a clean checkout? What is your platform and compiler?

@IOBYTE
Copy link
Copy Markdown
Contributor Author

IOBYTE commented Jan 18, 2023

Visual Studio gave 2 warnings for the size mismatch before this patch and none after.

@firewave
Copy link
Copy Markdown
Collaborator

The warning I posted is currently disabled. Along with the other conversion warnings. Fixes for those are not wanted.

To be fair the signed/unsigned warnings were/are really buggy which makes things messy. But the implicit shortening/truncation warnings are safe.

@IOBYTE
Copy link
Copy Markdown
Contributor Author

IOBYTE commented Jan 19, 2023

Where is the truncation with this patch?

    uint64_t flags() const {
        return mFlags;
    }
    void flags(const uint64_t flags_) {
        mFlags = flags_;
    }
    enum : uint64_t {
        ...
        fIsInline               = (1ULL << 32), // Is this a inline type
        fIsTemplate             = (1ULL << 33),
        fIsSimplifedScope       = (1ULL << 34), // scope added when simplifying e.g. if (int i = ...; ...)
        fIsRemovedVoidParameter = (1ULL << 35), // A void function parameter has been removed
        fIsIncompleteConstant   = (1ULL << 36),
        fIsRestrict             = (1ULL << 37), // Is this a restrict pointer type
        fIsSimplifiedTypedef    = (1ULL << 38),
        fIsFinalType            = (1ULL << 39), // Is this a type with final specifier
    };
    uint64_t mFlags;

Changing the argument and return type back to unsigned int is obviously wrong because you are throwing away bits that are being used. This even fixes a bailout because fIsSimplifiedTypedef was being discarded. This is fixing a real bug that also happens to be a real and valid warning. I don't understand what the problem is.

@firewave
Copy link
Copy Markdown
Collaborator

Ah, now I get it. The warning is with the code before your patch.

@firewave
Copy link
Copy Markdown
Collaborator

See also cppcheck-opensource/simplecpp#270.

@IOBYTE
Copy link
Copy Markdown
Contributor Author

IOBYTE commented Jan 21, 2023

I'm not a developer anymore so can someone else merge this?

@danmar danmar merged commit c3aa094 into cppcheck-opensource:main Jan 26, 2023
@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Jan 26, 2023

@IOBYTE good to see you hope you are well!

@IOBYTE
Copy link
Copy Markdown
Contributor Author

IOBYTE commented Jan 28, 2023

I'm doing good. Hope you and your family are too. Retirement is great but getting old sucks.

Sign up for free to 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.

4 participants