yonghong-song added inline comments.
================
Comment at: clang/include/clang/AST/TypeLoc.h:923
+ void initializeLocal(ASTContext &Context, SourceLocation loc) {}
+
+ QualType getInnerType() const { return getTypePtr()->getWrappedType(); }
----------------
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.
================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1144
+ SmallVector<llvm::Metadata *, 4> Annots;
+ auto BTFTagAttrTy = dyn_cast<BTFTagAttributedType>(PointeeTy);
+ while (BTFTagAttrTy) {
----------------
aaron.ballman wrote:
> aaron.ballman wrote:
> >
> This one may have been missed.
You mean:
```
auto *BTFTagAttrTy = dyn_cast<BTFTagAttributedType>(PointeeTy);
```
I missed it. Will fix in the next revision.
================
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);
+}
----------------
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.
Repository:
rG LLVM Github Monorepo
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
