Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace #include guards with #pragma once #9069

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

c4rlo
Copy link
Contributor

@c4rlo c4rlo commented Feb 5, 2023

  • Make the names of #include guards consistent across the project (with a special category for src/fdosecrets/*/*.h based on prevailing usage). I have left out headers that clearly originate from other projects.
  • Also make the format of the comment after the #endif consistent.
  • Remove one case of #pragma once, replacing it with an #include guard.

Per discussion below, this PR now replaces #include guards with #pragma once. This was done using the following script:

#!/usr/bin/python

import pathlib
import re


def replace(match, path, is_found):
    is_found[0] = True
    begin_newline = match.group(1)
    body = match.group(2)
    print(f"{path}: fixing")
    num_end_newlines = 0
    for c in reversed(body):
        if c == "\n":
            num_end_newlines += 1
        else:
            break
    if num_end_newlines > 1:
        body = body[:(1-num_end_newlines)]
    return (f"{begin_newline}#pragma once\n{body}")


def process(path):
    content = path.read_text()
    if re.search(r"^#pragma once", content, flags=re.MULTILINE):
        print(f"{path}: ok")
        return

    is_found = [False]
    processed = re.sub(
        r"(^|\n)#ifndef .+?\n#define .+?\n(.*\n)#endif.+?\n?$",
        lambda m: replace(m, path, is_found),
        content,
        count=1,
        flags=re.DOTALL,
    )
    if not is_found[0]:
        print(f"{path}: missing")
    else:
        path.write_text(processed)


def relevant_files():
    yield from pathlib.Path("src").walk()
    yield from pathlib.Path("tests").walk()


for dirpath, dirnames, filenames in relevant_files():
    if str(dirpath) == "src":
        dirnames.remove("thirdparty")
    for name in filenames:
        path = dirpath / name
        if path.suffix in (".h", ".h.cmake") and str(path) != "src/streams/qtiocompressor.h":
            process(path)

Testing strategy

TeamCity

Type of change

  • ✅ Refactor (significant modification to existing code)

@droidmonkey
Copy link
Member

Convince me why we shouldn't just switch everything over to #pragma once. @phoerious what are your thoughts?

@c4rlo
Copy link
Contributor Author

c4rlo commented Feb 5, 2023

Yeah, that was my other thought. But it's not standardised. I haven't tried to look into which compilers do or don't support it.

@droidmonkey
Copy link
Member

droidmonkey commented Feb 5, 2023

Every compiler supports it on platforms we support. The only hold out was GCC prior to version 3.4 which is ancient.

@codecov-commenter
Copy link

codecov-commenter commented Feb 5, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.83%. Comparing base (a02ddc7) to head (b8fb916).

Current head b8fb916 differs from pull request most recent head 9c9726b

Please upload reports for the commit 9c9726b to get more accurate results.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #9069      +/-   ##
===========================================
+ Coverage    63.74%   64.83%   +1.08%     
===========================================
  Files          362      342      -20     
  Lines        44733    44392     -341     
===========================================
+ Hits         28515    28778     +263     
+ Misses       16218    15614     -604     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@phoerious
Copy link
Member

I think most people would expect include guards and its what IDEs tend to auto generate. Besides that I don't really care what we use. Consistency would be nice.

@droidmonkey
Copy link
Member

droidmonkey commented Feb 18, 2023

I prefer #pragma once since it is (1) one line, (2) easy to understand, (3) doesn't require unique naming, (4) modern and hip 😉

@c4rlo do you mind making that switch?

@droidmonkey
Copy link
Member

Bumping this

@c4rlo
Copy link
Contributor Author

c4rlo commented Jun 22, 2024

I can have another look at this, yes. Happy to go with #pragma once.

I used the following script to do this:

    #!/usr/bin/python

    import pathlib
    import re

    def replace(match, path, is_found):
        is_found[0] = True
        begin_newline = match.group(1)
        body = match.group(2)
        print(f"{path}: fixing")
        num_end_newlines = 0
        for c in reversed(body):
            if c == "\n":
                num_end_newlines += 1
            else:
                break
        if num_end_newlines > 1:
            body = body[:(1-num_end_newlines)]
        return (f"{begin_newline}#pragma once\n{body}")

    def process(path):
        content = path.read_text()
        if re.search(r"^#pragma once", content, flags=re.MULTILINE):
            print(f"{path}: ok")
            return

        is_found = [False]
        processed = re.sub(
            r"(^|\n)#ifndef .+?\n#define .+?\n(.*\n)#endif.+?\n?$",
            lambda m: replace(m, path, is_found),
            content,
            count=1,
            flags=re.DOTALL,
        )
        if not is_found[0]:
            print(f"{path}: missing")
        else:
            path.write_text(processed)

    def relevant_files():
        yield from pathlib.Path("src").walk()
        yield from pathlib.Path("tests").walk()

    for dirpath, dirnames, filenames in relevant_files():
        if str(dirpath) == "src":
            dirnames.remove("thirdparty")
        for name in filenames:
            path = dirpath / name
            if path.suffix in (".h", ".h.cmake") and str(path) != "src/streams/qtiocompressor.h":
                process(path)
@c4rlo c4rlo marked this pull request as ready for review June 23, 2024 21:31
@c4rlo c4rlo changed the title Fix #include guard names Replace #include guards with #pragma once Jun 23, 2024
@c4rlo
Copy link
Contributor Author

c4rlo commented Jun 23, 2024

Updated to #pragma once.

Comment on lines +1 to +2
/*
* Copyright (C) 2021 Team KeePassXC <[email protected]>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The diff with this file and a few others is that Windows-style line endings were converted to Unix-style line endings.

Copy link
Member

Choose a reason for hiding this comment

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

Not surprised these aren't fully normalized

@droidmonkey
Copy link
Member

Cool script!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants