Can we put the __STDC_VERSION__ case first (actually, second, after the Doxygen case)?

Can we put the __STDC_VERSION__ case first (actually, second, after the Doxygen case)?

@pitrou please elaborate.

Hi @jensenrichardson I don't feel comfortable with this change without a pre-processor-based check for the version of the intel compiler and research about when that changed.
Alternatively, we can drop the whole block on intel and leave inlining to the compiler altogether, at least when using the intel one.

@hartwork Unfortunately, the historical Intel c++ compiler documentation is non-existant (I have done lots, and lots of searching), so I can't find where it changed. However, I have gone back through the intel developer forums, and I did a search for __forceinline and I was able to find examples of it's usage all the way back to 2003 (where I stopped), which is well before the original commit that added the intel compiler options to Uriparser. Because it seems to be supported so far back, I am assuming that it was perhaps deprecated declaration at the time it was added. Furthermore, I can find no reference whatsoever to __force_inline on the Intel Developer Forum. So, I don't think there should be no problems at all with compatibility.

What do you think? I recognize it's still not ideal, but I think it is highly unlikely to negatively affect anyone since the Intel compile stack is fairly rare, it seems to be supported for a long time, and without it, newer stacks cannot build the Uriparser at all. Sorry for the long comment, I've just been debugging this for like four days straight :)

@pitrou please elaborate

I mean that the __STDC_VERSION__ block should match all C99 compilers. In that case you don't need compiler-specific cases.
The Intel compiler supports C99 when given the right options: https://software.intel.com/content/www/us/en/develop/articles/c99-support-in-intel-c-compiler.html

Thanks for your replies. I'm good with either of these two options:

a)

# if __INTEL_COMPILER >= 1900
#  define URI_INLINE __forceinline
# else
#  define URI_INLINE __force_inline
# endif

b)

Drop the whole block specific to Intel compiler.

Both options should fix compilation for latest Intel compiler 19.0.5. I'm making an assumption here about use of 19.x.x because non of the linked reports I saw mention which version of ICC was used and causing trouble; did I miss something?

Please let me know how happy or unhappy you are with these options.

Hi Sebastian,

Sorry I never mentioned what version I'm using. I'm using (some variant of 18). I think that if we took this approach we'd have to go back to at least version 18.

But the larger issue is that I honestly think that we might be better served with just # define URI_INLINE __forceinline because I have looked everywhere and I can't find any reference of the intel compiler using __force_inline only ever __forceinline. However I can see that MSVC has used it, so I think that it might have been a mistake when it was originally defined. But, since the Intel compiler is so rare, it was just never caught (or no-one bothered to fix it) until now.

I understand, thanks for elaborating. If we go this route, I'd ask to drop the line /* EDIT 11/5/20. Intel changed __force_inline to __forceinline */ from the pull request as that's just not the right place for change logs with regard to this codebase. I'm happy to add a related entry to the central change log myself on top in a new commit.

@pitrou what do you think about #93 (comment) ?

Nope that makes sense. I removed it.

De-facto squash-merged to master now. Thanks again, closing.