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

Fixes/lib (warnings compilation) #495

Merged
merged 6 commits into from
Sep 6, 2024
Merged

Conversation

Sazerac4
Copy link
Contributor

@Sazerac4 Sazerac4 commented Sep 5, 2024

  • Clang-format "SortInclude" disable: Sorting include can broke the compilation (order is important)
  • Formatting source files according to rules.
  • Fixes macros called with wrong argument "pFmt" instead of "fmt" . Add TRICE_UNUSED
  • Add missing TRICE_INLINE on float/double functions

Question:

  • trice*.c and trice*.h files are now filtered from formatting. how formatiing theses files are problematic ?
  • What is the purpose to use TRICE_INLINE instead of static inline. Some compilers got better attributes ?

- Fix trice function don't pass fmt to macros ( -Wunused )
- Fix undef preprocessor ( -Wundef )
@Sazerac4
Copy link
Contributor Author

Sazerac4 commented Sep 5, 2024

  • Set TRICE_DIAGNOSTICS to 0 fail to compile. Some macros uses variable that are not declared. Fixed

@Sazerac4 Sazerac4 marked this pull request as draft September 5, 2024 14:56
@rokath
Copy link
Owner

rokath commented Sep 5, 2024

Question:
trice*.c and trice*.h files are now filtered from formatting. how formatting theses files are problematic ?

The mixed indent of macros and code is manually adapted. To not spoil the code with // clang-format off/on I excluded them for now to get automatic formatted, especially in the assumption, that this code will not see much changes.

I am not sure about the best way to handle that. Any proposals?

Edit: Maybe reordering the code that way, that only one code segment needs to be excluded from auto formatting.

Question:
What is the purpose to use TRICE_INLINE instead of static inline. Some compilers got better attributes ?

If I remember right, I have seen C compilers with some weird function inline syntax. To handle such cases, the user can define the TRICE_INLINE macro accordingly.

@rokath rokath marked this pull request as ready for review September 5, 2024 21:27
@Sazerac4
Copy link
Contributor Author

Sazerac4 commented Sep 6, 2024

I am not sure about the best way to handle that. Any proposals?

The problem is mixing preprocessor directives indenting with statements (if, for, switch, ...) indenting that might yield very misleading looking code when preprocessor is heavy used with code.
example:

void foo(void) {
#if TEST1 == 0
	#if TEST2 == 0
		if (true) {  // This if is on the wrong indentation
	#endif
#endif
		for (int i = 0; i < 10; ++i) {
			// do something
		}
#if TEST1 == 0
	#if TEST2 == 0
		}  // This brace is on the wrong indentation
	#endif
#endif
}

I think we can reverse the logic, set IndentPPDirectives: None and add manual indentation when it is pertinent with clang-format off.
Edit: It might be worth it. Most styles do not indent directives.

Edit: Maybe reordering the code that way, that only one code segment needs to be excluded from auto formatting.

I think you shouldn't code based on formatter (most of the time). We add unnecessary complexity when programming

@Sazerac4 Sazerac4 marked this pull request as draft September 6, 2024 11:29
@Sazerac4 Sazerac4 marked this pull request as ready for review September 6, 2024 11:29
@Sazerac4 Sazerac4 force-pushed the fixes/tests_lib branch 7 times, most recently from be0fc10 to bc10a84 Compare September 6, 2024 12:19
@rokath rokath merged commit bb27fd0 into rokath:master Sep 6, 2024
@rokath
Copy link
Owner

rokath commented Sep 6, 2024

Maybe we keep the trice8|16|32|64.c|h files inside .clang-format-ignore. These files would loose readability for my taste when re-formatted with IndentPPDirectives: None. But I have no strong opinion on this because they then formatted differently to trice.c|h. On trice.c|h I agree to drop the pre-processor macros indent.
Edit: https://stackoverflow.com/questions/2975330/how-do-you-indent-preprocessor-statements

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.

2 participants