lib/crash.cpp: Some checking (bounds etc. via the new `nelems` and `azzert`).

[DoctorNoobingstoneIPresume]

[onf-cla-manager[bot]]

Hi @DoctorNoobingstoneIPresume, this is the ONF bot 🤖 I'm glad you want to contribute to our projects! However, before accepting your contribution, we need to ask you to sign a Contributor License Agreement (CLA). You can do it online, it will take only a few minutes:

✒️ 👉 https://cla.opennetworking.org

After signing, make sure to add your Github user ID DoctorNoobingstoneIPresume to the agreement.

For more information or help:" https://wiki.opennetworking.org/x/BgCUI

[mihaibudiu]

Even if you don't like our cpplint rules, your code still has to pass the check to be merged. You can submit a PR against cpplint if you feel strongly about it, it's just a python program.

[DoctorNoobingstoneIPresume]

Even if you don't like our cpplint rules, your code still has to pass the check to be merged. You can submit a PR against cpplint if you feel strongly about it, it's just a python program.

Thank you for the suggestion (also appliable to my other pull requests (https://github.com/p4lang/p4c/pull/3296, https://github.com/p4lang/p4c/pull/3295, https://github.com/p4lang/p4c/pull/3294).

FWTAW (for whaterver they are worth), my current thoughts are:

• If one adds a new source file, a different style might be allowed (if acceptable to the other contributors).
• If I simply add a line inside an existing function, it is probably better to change it to match the style of the other lines. I intend to modify my PR's as such.
• Generally, I feel there is a scale to allowing different style: (1) extra line(s) in existing function; (2) extra function(s) in existing class/namespace/file; (3) extra class in existing namespace/file; (4) extra file in existing folder; (5) extra folder in current project/repo; ... (9) external application (using the compiler) written on planet in different solar system (but still targetting x86, of course); ... (19) ... ARM and other architectures...

Regarding the order of #included headers, even if other persons end up agreeing with https://github.com/p4lang/p4c/pull/3295#issuecomment-1122181222, I have thought and currently I feel that for existing #include directives, it may be better to keep them as they are, in order not to affect too much the output of (some common incantations of) the git log -p, git blame etc. -- but instead allow different orders (if agreed upon with regard to advantages) for new #include directives (at least in new files).

=> We currently might not want to impose different constraints on existing code, but instead to relax constraints or modify them for new code.

• Currently, I think we have // NOLINT comment to suppress cpplint for one line.
• We might wish to support something like // NOLINT (WhiteSpace, IncludeOrder) { and // NOLINT } (yes, with opening brace and closing brace) in order to suppress (certain options of) cpplint for a larger block.

I intend to learn how to do that, but I need some time (really to learn the basics of Python).

In the meanwhile, I think opinions from others will be very good (I am just a new person here, with ~zero contribution). E.g.: "Yes, sometimes the style constraints are too restrictive and parts of CodingStandardPhilosophy.md are subjective." or "No, the current order of headers is better because..." or "A better way to suppress cpplint might be..." or "Be gone, Intruder !!".

[mihaibudiu]

Contributions are appreciated, but I have too much other stuff to do to fight with cpplint. You can use // NOLINT indeed.

[fruffy]

Part of the reason these linters exist is for style consistency and to offload some of the review work to automated tools. I think we should have even more linters, particularly ones that enforce code formatting. I might add clang-format at some point. We have been using it successfully for other P4C projects.

Afaik the order this project uses is defined by the Google style guide. I personally also prefer the LLVM style, but this repo has committed to this particular guideline.

Even then, I have not encountered a case where the include order restriction really causes a substantial problem. If there really is a case where the include order matters, we can always use NOLINT. However, usually these scenarios indicate that there is something funky going on.

[DoctorNoobingstoneIPresume]

Contributions are appreciated, but I have too much other stuff to do to fight with cpplint. You can use // NOLINT indeed.

Thank you. IIUC, we have //NOLINT for individual lines, but not for blocks (i.e. groups of lines). I hope to be able to implement that.

[DoctorNoobingstoneIPresume]

Part of the reason these linters exist is for style consistency and to offload some of the review work to automated tools. I think we should have even more linters, particularly ones that enforce code formatting. I might add clang-format at some point. We have been using it successfully for other P4C projects.

Automating such tasks is good !

Afaik the order this project uses is defined by the Google style guide. I personally also prefer the LLVM style, but this repo has committed to this particular guideline.

I have not found the rationale behind the #include order in the Google Style guide.

FWIW, I have given my rationale for the different order: https://github.com/p4lang/p4c/pull/3295#issuecomment-1122181222.

Even then, I have not encountered a case where the include order restriction really causes a substantial problem. If there really is a case where the include order matters, we can always use NOLINT. However, usually these scenarios indicate that there is something funky going on.

Let us suppose we want to modify the behaviour of libgc headers (or the headers of other dependency) by some #define -- and we place those #define-itions in a project-specific header that we expect our source code to #include before/instead-of #include-ing the libgc etc. header.

This is not going to work smoothly because of the current order of #include's -- i.e. other files in our project might #include the dependency header before the #define-ing header.

Also the self-sufficiency of headers is less checked with the current order of #include's.

(I am sorry for repeating myself.)

[fruffy]

I have not found the rationale behind the #include order in the Google Style guide.

Yeah I do not think there is one provided. Afaik the reason is a) consistent style b) to throw an error when you redefine something in a local file (since you can change those).

Regarding your example, the easiest solution it to likely mandate to always include the customized include header instead of the gc system header. Or to have the #define directive be explicit with every include.

Personally I am not opposed to changing the include style. In fact, I would not mind getting rid of cpplint and using the better-maintained clang-tidy instead.

However, there does not seem sufficient justification for that right now. Does #3295 not compile if you move the include up?

[DoctorNoobingstoneIPresume]

Thank you, @fruffy. Partial answer to just your last question:

However, there does not seem sufficient justification for that right now. Does #3295 not compile if you move the include up?

I agree. I believe it does compile, however I am going to check exactly and get back soon !