#if DOESN’T test for something that looks like a 0 or “anything else”, but evaluates the expression given (with unknown symbols being treated as 0) and sees if it has a VALUE of 0, or a non-zero value. PortSET_INTERRUPT_MASK_FROM_ISR, when implemented, will likely be an inline assembly expression, which the preprocessor will just not see as an expression.
Again, we are not in a major disagreement here, you are splitting hairs in two even though the basic question has already been answered. I do not know why you keep doing that, but so it be.
Again, in the scenario
file1.h:
#if not defined MYSYMBOL
#define MYSYMBOL 0
#endif
file2.h:
#if MYSYMBOL
< do something involving MYSYMBOL>
#else
< do something not involving MYSYMBOL>
#endif
it is ensured that if and only if MYSYMBOL has been set by an external module, the first clause in file2.h will catch in and use MYSYMBOL as intended. Thus, replacing #ifdef by #if in atomic.h qualifies as a sufficient solution to the bug reported by Sebastian (but see below).
Again, no good coding style as MYSMBOL is “overloaded” which obfuscates its meaning; furthermore:
-
If ever MYSYMBOL is not a preprocessable identifier but, say, a function declaration, there will be resulting build errors. Again, which existing ports expose the problem, in particular, which of the Cortex-M ports you mentioned earlier are affected?
-
The #if shield is required for every access to MYSYMBOL which is dangerous as that my be forgotten in a larger code base.
Most port implements portSET_INTERRUPT_MASK_FROM_ISR with function-like macro. In kernel, it is also used as function-like macro. If the usage in kernel remains, the macro definitions we want to distinguish are
#define portSET_INTERRUPT_MASK_FROM_ISR() ulPortRaiseBASEPRI()
#define portSET_INTERRUPT_MASK_FROM_ISR() 0
These definitions can’t be caught by the #if portSET_INTERRUPT_MASK_FROM_ISR condition. #define portSET_INTERRUPT_MASK_FROM_ISR 0 also breaks function-like macro usage in kernel.
Port can define portSET/CLEAR_INTERRUPT_MASK_FROM_ISR to specify it has nested interrupt. Defining portHAS_NESTED_INTERRUPT in FreeRTOS.h seems reasonable to me since these macros will be stubbed in FreeRTOS.h. It is not easy to tell if these macros are usable in other header files. I create this PR for discussion.
Why not?
#define portSET_INTERRUPT_MASK_FROM_ISR() 0
#ifdef portSET_INTERRUPT_MASK_FROM_ISR
#error “ifdef catches”
#endif#if portSET_INTERRUPT_MASK_FROM_ISR
#error “if catches”
#else
#error “if preprocessor does not catch”
#endif
=>
ifdef catches
if preprocessor does not catch
(tested a minute ago on gcc 11.3.0-1ubuntu1~22.04)
Try it with portSET_INTERRUPT_MASK_FROM_ISR set to something like is used in a port that supports it.
If it is defined like
#define portSET_INTERRUPT_MASK_FROM_ISR ulPortRaiseBASEPRI()
Then #if portSET_INTERRUPT_MASK_FROM_ISR becomes
#if ulPortRaiseBASEPRI()
Which since ulPortRaiseBASEPRI isn’t defined to the preprocessor becomes equivalent to
#if 0()
Which is a syntax error, NOT a “true” preprocessor conditional.
Apologies, but that does not appear to make sense. A port that maps portSET_INTERRUPT_MASK_FROM_ISR to ulPortRaiseBASEPRI is also in charge of defining/declaring ulPortRaiseBASEPRI (that is a local identifier to the port, not anything external - must also be the case because BASEPRI is a concept known to individual MCUs, not FreeRTOS in general) - in the ports I have access to right now, it is generally declared as an inline function. Why would a port map portSET_INTERRUPT_MASK_FROM_ISR to something unknown/undefined/inaccessible? I could see your point if the map target would be something port independent, but that would defeat the purpose of a port.
Iow, IF a port decides to implement portSET_INTERRUPT_MASK_FROM_ISR, the port would be in charge of fully defining it with regard to the target MCU, so the scenario you picture would be an error in the port and thus justly raise a compiler error.
As a side note,
#define portSET_INTERRUPT_MASK_FROM_ISR ulPortRaiseBASEPRI()
in your last reponse is probably a typo as portSET_INTERRUPT_MASK_FROM_ISR
is generally defined as a parametrized macro, so it should probably read
#define portSET_INTERRUPT_MASK_FROM_ISR() ulPortRaiseBASEPRI()
which would resolve your syntax error, ironically making everything even worse, as now, we would be facing run time errors similar to the one Sebastian discovered, and runtime errors are always worse than build errors.
To sum it up, using
#if
in the scenario you picture would raise a build error which uncovers an underlying design error, which is good.
But for the last time - I am tired of you repeatedly digging out whatever you can find (fringe or illegal scenarios or irrelevant details) for the sole purpose of proving others wrong. It is a waste of everybody’s time. We are in full agreement about pretty much everything, and I never claimed that replacing the #ifdef with an #if in atomic.h would be a satisfactory, let alone a good solution, just a semantically correct solution. If I am wrong (which of course is very possible as nobody is perfect), the good news is that I would be proven wrong during system build (which may, as a side effect, reveal so far undiscovered bugs in individual ports).
But ulPortRasieBASEPRI will likely not be a preprocessor macro, and thus not seen by the #if statement
#if is different than the if statement, #if only accesses preprocessor macros, not all symbols defined.
Note, the Build Error that you are talking about will occur in EVERY port that actually defines portSET_INTERRUPT_MASK_FROM_ISR(), as the definition of that will NEVER resolve to a preprocessor expression that can be evaluated into a value, since, by definition, it will need a generate a run time value.
I also run the following tests with GCC.
#define portSET_INTERRUPT_MASK_FROM_ISR() 0
#ifdef portSET_INTERRUPT_MASK_FROM_ISR
#error “ifdef catches”
#endif
#if portSET_INTERRUPT_MASK_FROM_ISR
#error “if catches”
#else
#error “if preprocessor does not catch”
#endif
extern unsigned int ulPortRaiseBASEPRI( void );
#define portSET_INTERRUPT_MASK_FROM_ISR() ulPortRaiseBASEPRI()
#ifdef portSET_INTERRUPT_MASK_FROM_ISR
#error “ifdef catches”
#endif
#if portSET_INTERRUPT_MASK_FROM_ISR
#error “if catches”
#else
#error “if preprocessor does not catch”
#endif
These two tests show the same result.
main.c:6:2: error: #error “ifdef catches”
#error “ifdef catches”
^~~~~
main.c:12:2: error: #error “if preprocessor does not catch”
#error “if preprocessor does not catch”
^~~~~
These #ifdef and #if tests are not helpful to know which macro is usable. It is probably easier to tell before these marcos are defined in FreeRTOS.h.
Since the macro was defined with the (), the #if needs the () too, or it is looking for a non-function like macro.
ok, point taken and accepted. Thanks for pointing this out, and apologies to you and Richard for dismissing your just reservations. I was aware of the C preprocessor being very unhygienic, but it never fails to surprise me.
[there is a lot to unpick here - we can update the bug report and take any remedial action if needed once concluded]