Hi Cliff,
FYI I've just this minute merged all the latest updates to proton-c onto
the branch I'm using for the JavaScript bindings and have rebuilt
everything, the changes that you've done for this Jira work beautifully
for me too!!! Many thanks, you're a star.
Cheers,
Frase
On 28/02/14 08:48, Cliff Jansen wrote:
Sorry for the delay.
I played a bit with emcripten to see if there was some
misunderstanding going on, but it seems pretty clear that clang knows
exactly what is being passed in via the va_arg call and just plain
refuses to deal with a struct (at least for some architectures).
Consequently the original patch which restricts va_arg processing to
simple types seems to me the most robust and portable solution. I
will proceed with that for 0.7.
Cliff
a bit indicates to me that clang is not somehow confused about
On Thu, Feb 27, 2014 at 9:01 AM, Fraser Adams
fraser.ad...@blueyonder.co.uk wrote:
Hi again Cliff,
Have you made any progress on this?
Cheers,
Frase
On 30/01/14 02:31, Cliff Jansen wrote:
Well thanks for all that. I'll certainly take another look and report
back before committing anything. Giving a compiler family indigestion
is certainly to be avoided.
Cliff
On Tue, Jan 28, 2014 at 11:29 AM, Fraser Adams
fraser.ad...@blueyonder.co.uk wrote:
Hi, me again Cliff.
I've only had time to recheck this against my test case, which is
representative of what I see for real.
Doing:
int pn_data_vfill2(const char *fmt, va_list ap)
{
// Process the PROPERTIES constant - this seems OK
uint64_t prop = va_arg(ap, uint64_t);
printf(prop = %llu\n, prop);
{
pn_bytes_t bytes = va_arg(ap, pn_bytes_t);
printf(pn_data_vfill z, bytes.size = %zu, bytes.start = %s\n,
bytes.size,
bytes.start);
}
// Process the char* returned by pn_string_get()
{
char *start = va_arg(ap, char *);
size_t size = strlen(start);
printf(pn_data_vfill size = %zu\n, size);
printf(pn_data_vfill string = %s\n, start);
}
return 0;
}
E.g. the both passing and retrieving structs approach of your second
approach actually doesn't even compile for me with LLVM le32, I get
error:
cannot compile this aggregate va_arg expression yet
pn_bytes_t bytes = va_arg(ap, pn_bytes_t);
^~
/home/fadams/emscripten/system/include/libc/stdarg.h:15:25: note:
expanded
from
macro 'va_arg'
#define va_arg(v,l) __builtin_va_arg(v,l)
^
1 error generated.
ERRORroot: compiler frontend failed to generate LLVM bitcode, halting
make[2]: *** [CMakeFiles/test_varargs.dir/test_varargs.o] Error 1
make[1]: *** [CMakeFiles/test_varargs.dir/all] Error 2
make: *** [all] Error 2
Though it *does* work if I do EMCC_LLVM_TARGET=i386-pc-linux-gnu make
So in a nutshell:
1) if I fudge the LLVM front end pure struct, put struct retrieve
discrete
and pure discrete values all work correctly.
2) with the standard emscripten le32 LLVM front end (which is what is
recommended)
* pure struct (as above) fails to compile
* put struct, retrieve discrete (as with the original Proton code) gives
the wrong results - that messed with my head :-)
* pure discrete values (as with your pn_string_size(msg-user_id),
pn_string_get(msg-user_id),) patch works correctly.
So I'd *really* like it if you could go down the path of your original
patch
'cause that would fix what you've been seeing and let me compile the
JavaScript stuff without needing the environment tweaked (which is an
accident waiting to happen). I hope that you are agreeable to that?
I think that it would probably be worth putting a comment in the call to
pn_data_fill and within the pn_data_vfill() 'z' case body to document
that
it's safer to pass individual entries to va_arg to avoid upsetting some
compilers - it's not entirely an *obvious* thing really :-D
Cheers,
Frase
On 27/01/14 21:21, Cliff Jansen wrote:
Thanks for the Javascript related info.
Fraser: can you test if the review board patch (with the struct in
and out strategy) works in your case with the unhacked llvm setup?
If that works then I'll go ahead and check it in.
If it fails, please try the first patch. If that works, we will just
have to conclude that compilers have trouble with stucts in this case
and fall back to passing the two basic types. That should be safe to
work in the greatest number of cases.
Many thanks.
On Mon, Jan 27, 2014 at 1:13 PM, Fraser Adams (JIRA) j...@apache.org
wrote:
[
https://issues.apache.org/jira/browse/PROTON-488?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13883314#comment-13883314
]
Fraser Adams commented on PROTON-488:
-
Oh to be clear
either struct in and out, or separate size_t and char* in and out
what I meant in my previous comment was that the separate size_t and
char* in and out is what LLVM le32 is
