aaron.ballman added inline comments.
================
Comment at: clang/include/clang/AST/TypeLoc.h:923
+ void initializeLocal(ASTContext &Context, SourceLocation loc) {}
+
+ QualType getInnerType() const { return getTypePtr()->getWrappedType(); }
----------------
yonghong-song wrote:
> aaron.ballman wrote:
> > Do we also need something like this?
> > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/TypeLoc.h#L1187
> I didn't see getLocalDataSize() in AttributedTypeLoc so it is not needed for
> AttributedTypeLoc. BTFTagAttributedTypeLoc usage is very similar to
> AttributedTypeLoc, so I think we are fine here.
The main difference is that `AttributedLocInfo` has a member and
`BTFTagAttributedLocInfo` is empty. This is why I think it's closer to an
`AdjustedLocInfo` object which also is an empty struct.
================
Comment at: clang/test/Sema/attr-btf_type_tag.c:29
+int g1 = _Generic((int *)0, int __tag1 *: 0);
+int g2 = _Generic((int __tag1 *)0, int *: 0);
+
----------------
I'd like a third test case:
```
int g3 = _Generic(0, int __tag1 * : 0, int * : 0, default : 0); //
expected-error {{something about duplicate compatible associations}}
```
================
Comment at: clang/test/Sema/attr-btf_type_tag.c:31-37
+// The btf_type_tag attribute will be ignored during overloadable type matching
+void __attribute__((overloadable)) bar1(int __tag1 *a);
+void __attribute__((overloadable)) bar2(int *a);
+void foo2(int *a, int __tag1 *b) {
+ bar1(a);
+ bar2(b);
+}
----------------
yonghong-song wrote:
> aaron.ballman wrote:
> > This test looks confused -- those functions have different names, so
> > they're not overloading one another. Even if they were named with the same
> > identifier, it wouldn't be sufficient to test which gets called (you can
> > redeclare overloads as in https://godbolt.org/z/aebMbY1aM).
> >
> > I think this case needs a codegen test to see which function is actually
> > dispatched to.
> sorry for confusion. the test is bad. For the example in the above
> godbolt.org link, we have
> ```
> __attribute__((overloadable)) void func(int a);
> __attribute__((overloadable)) void func(int a);
> ```
> Such multiple declarations are actually okay since we allow duplicated
> declarations. As you mentioned, we really want to know which function
> definition it picks with overloadable attribute.
> I tried a few examples:
> ```
> $ cat test.c
> #if 1
> #define __attr __attribute__((btf_type_tag("tag1")))
> #else
> #define __attr __attribute__((noderef))
> #endif
>
> void bar1(int __attr *);
> void bar2(int *);
> void isdigit1(int __attr *a) __attribute__((overloadable)) {
> bar1(a);
> }
> void isdigit1(int *a) __attribute__((overloadable)) {
> bar2(a);
> }
>
> void foo(int __attr *a, int *b) {
> isdigit1(a);
> isdigit1(b);
> }
>
> $ clang -g -std=c2x test.c -S -O2 -emit-llvm -DUSE_TAG
> test.c:12:6: error: redefinition of 'isdigit1'
> void isdigit1(int *a) __attribute__((overloadable)) {
> ^
> test.c:9:6: note: previous definition is here
> void isdigit1(int __attr *a) __attribute__((overloadable)) {
> ^
> 1 error generated.
> $ clang -g -std=c2x test.c -S -O2 -emit-llvm
> test.c:12:6: error: redefinition of 'isdigit1'
> void isdigit1(int *a) __attribute__((overloadable)) {
> ^
> test.c:9:6: note: previous definition is here
> void isdigit1(int __attr *a) __attribute__((overloadable)) {
> ^
> 1 error generated.
> $
> ```
>
> so just attribute itself cannot differentiate overloadable functions. In the
> above test.c, if I changed one of isdigle1 and corresponding bar?() function
> from 'int' to 'float', compilation can succeed.
>
> So I will add overloadable failure and success case in Sema/ directory. I am
> not sure whether a codegen case is needed for the case like
> ```
> void bar1(float __attr *);
> void bar2(int *);
> void isdigit1(float __attr *a) __attribute__((overloadable)) {
> bar1(a);
> }
> void isdigit1(int *a) __attribute__((overloadable)) {
> bar2(a);
> }
> ```
> or not. But if you think it is necessary, I am happy to add it.
Okay, I am fine with that behavior. CodeGen tests are only necessary if the
overloads can actually be defined -- since they can't, I think the Sema tests
are sufficient to show that these are redeclarations and not independent
overloads.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120296/new/
https://reviews.llvm.org/D120296
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
