moved part of `FileDataCache` implementation into source file by firewave · Pull Request #651 · cppcheck-opensource/simplecpp

firewave

Choose a reason for hiding this comment

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

should it be static also? the declaration is static as far as I can tell.

Choose a reason for hiding this comment

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

Yes. That should have been caught by the compiler or clang-tidy though.

Choose a reason for hiding this comment

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

I am confused by your comments.. do you plan to add the static keyword or is there some reason to not add it?

Choose a reason for hiding this comment

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

It is already static in the forward declaration - see the other comment thread.

Choose a reason for hiding this comment

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

so the reason you don't write static here is because that would be redundant?

To me it would feel preferable to match the prototype and write the static keyword even if it's technically redundant.

Choose a reason for hiding this comment

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

Now I am finally getting what your are aiming for. I have never seen code written like that. Usually static (like extern) is only applied to the declaration. It is valid code though and does not even trigger a warning (which is surprising to me).

Choose a reason for hiding this comment

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

I think the reason the extern is usually not written is that it's totally redundant. the function is extern with or without the keyword..

I want that we have matching declaration and definition. I don't want that any static or typename or variablename differ or anything even if it does not technically matter.

Choose a reason for hiding this comment

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

	bool getFileId(const std::string &path, FileID &id)
	static bool getFileId(const std::string &path, FileID &id)

for methods the static must be left out for the out of line definition. but that doesn't mean it should be left out here.

Choose a reason for hiding this comment

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

Done.