riccibruno added inline comments.

================
Comment at: clang/lib/Sema/SemaInit.cpp:4539
+    if (CXXRecordDecl *T2RecordDecl =
+        dyn_cast<CXXRecordDecl>(T2RecordType->getDecl())) {
+      const auto &Conversions = T2RecordDecl->getVisibleConversionFunctions();
----------------
ArcsinX wrote:
> riccibruno wrote:
> > I cannot reproduce this with the provided test case. Can you explain why 
> > `T2RecordDecl` would not be a `CXXRecordDecl` since we should only get 
> > there with c++?
> You can reproduce it with
> ```
> bin/clang -xc --target=arm-unknown-gnu -c ../clang/test/Sema/init-ref-c.c
> ```
> 
> We can get there with C in case of using builtins declared as functions with 
> reference parameters (e.g. `__builtin_va_end()`)
Okay I see it now. Thank you.

I think it would be better to only call `TryRefInitWithConversionFunction` in 
C++ mode since conversion functions don't exist in C (only the first call on 
line 4773).


================
Comment at: clang/lib/Sema/SemaInit.cpp:4706
                                            Qualifiers T2Quals,
                                            InitializationSequence &Sequence) {
   QualType DestType = Entity.getType();
----------------
Can you add a comment here explaining that we can get there in C with some 
builtins?


================
Comment at: clang/lib/Sema/SemaInit.cpp:4773
     if (RefRelationship == Sema::Ref_Incompatible && T2->isRecordType() &&
         (isLValueRef || InitCategory.isRValue())) {
       ConvOvlResult = TryRefInitWithConversionFunction(
----------------
Here


================
Comment at: clang/test/Sema/init-ref-c.c:8
+}
\ No newline at end of file

----------------
I think it would be better to name this test `varargs-arm.c` since this bug can 
only happen with the varargs-related builtins (`__builtin_addressof` takes a 
reference but has custom type-checking).

Additional test case:
```
const __builtin_va_list cva;
__builtin_va_end(cva); // expected-error {{binding reference of type 
'__builtin_va_list' to value of type 'const __builtin_va_list' drops 'const' 
qualifier}}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82805/new/

https://reviews.llvm.org/D82805



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to