echuraev added a comment. In https://reviews.llvm.org/D28136#634356, @Anastasia wrote:
> This has been discussed during the initial review for the header: > http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160425/157187.html > > The main issue is after preprocessing the header the original function name > is no longer available in diagnostics reported. The spec defines as_type as a > builtin function and not a macro. Additionally your patch would allow as_type > to be used with extra type (not only those defined in spec). Also I don't see > the problem to implement as_type with just simply calling a builtin. It > should be inlined later anyways. I think that this patch is really necessary because in some cases previous implementation doesn't give a diagnostic. Please see test case that I have added to this review. With current implementation //char// variable will be cast to //int//. You can see the following part of llvm ir: %0 = load i8, i8* %src, align 1 %conv = sext i8 %0 to i32 %call = call i32 @_Z6as_inti(i32 %conv) #2 So there is a bug and we didn't get error that the size of char isn't equal to size of int. Program is compiled without any problems. If as_type functions will be defined in macro then we will have the following message: error: invalid reinterpretation: sizes of 'float' and 'char' must match int dst = as_int( src ); ^~~~~~~~~~~~~~~ ././llvm/tools/clang/lib/Headers/opencl-c-common.h:6615:21: note: expanded from macro 'as_int' #define as_int(x) __builtin_astype((x), int) ^ ~~~ https://reviews.llvm.org/D28136 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
