balazske marked an inline comment as done.
balazske added inline comments.
Comment at: clang/lib/AST/Expr.cpp:1386
QualType CalleeType = Callee->getType();
+
if (const auto *FnTypePtr = CalleeType->getAs()) {
hokein wrote:
> nit: looks like an accident
This revision was automatically updated to reflect the committed changes.
balazske marked an inline comment as done.
Closed by commit rG6e5199104914: [clang][AST] Handle overload callee type in
CallExpr::getCallReturnType. (authored by balazske).
Changed prior to commit:
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
thanks, looks good!
Comment at: clang/lib/AST/Expr.cpp:1386
QualType CalleeType = Callee->getType();
+
if (const auto *FnTypePtr = CalleeType->getAs()) {
balazske marked an inline comment as done.
balazske added inline comments.
Comment at: clang/unittests/Tooling/SourceCodeTest.cpp:630
+ f(t);
+ // CalleeType in getCallReturntype is Overload and dependent
+}
hokein wrote:
> balazske wrote:
> > hokein wrote:
>
balazske updated this revision to Diff 336333.
balazske added a comment.
Rebase, changed test according to review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95244/new/
https://reviews.llvm.org/D95244
Files:
clang/lib/AST/Expr.cpp
hokein added inline comments.
Comment at: clang/unittests/Tooling/SourceCodeTest.cpp:630
+ f(t);
+ // CalleeType in getCallReturntype is Overload and dependent
+}
balazske wrote:
> hokein wrote:
> > CalleeType is not a specific term in `getCallReturnType`,
balazske marked 3 inline comments as done.
balazske added a comment.
Ping.
I am not sure how to the test should be changed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95244/new/
https://reviews.llvm.org/D95244
balazske updated this revision to Diff 332686.
balazske added a comment.
Fixed according to the comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95244/new/
https://reviews.llvm.org/D95244
Files:
clang/lib/AST/Expr.cpp
balazske added inline comments.
Comment at: clang/lib/AST/Expr.cpp:1387
+
+ auto CreateDependentType = []() {
+ASTContext::GetBuiltinTypeError Error;
hokein wrote:
> this can be removed, dependent type is a builtin type, so you could just use
>
hokein added a comment.
This looks like a reasonable solution to me -- if we don't know the function
call result type (e.g. dependent-type callExpr in template context), modeling
it as a dependent type.
Comment at: clang/lib/AST/Expr.cpp:1387
+
+ auto CreateDependentType =
dblaikie added a comment.
Added a couple of other folks who might have more context on the tooling
front...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95244/new/
https://reviews.llvm.org/D95244
___
martong added reviewers: aaron.ballman, riccibruno, dblaikie.
martong added subscribers: aaron.ballman, dblaikie.
martong added a comment.
Hi @aaron.ballman @riccibruno @dblaikie, I am adding you as a reviewer because
it seems you have great experience with Clang's AST.
Could you please take a
balazske updated this revision to Diff 329647.
balazske added a comment.
rebase, split the test case
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95244/new/
https://reviews.llvm.org/D95244
Files:
clang/lib/AST/Expr.cpp
martong added inline comments.
Comment at: clang/lib/AST/Expr.cpp:1406
+ } else if (CalleeType->isDependentType() ||
+ CalleeType->isSpecificPlaceholderType(BuiltinType::Overload)) {
+return CreateDependentType();
Can't we create a built in
martong added a comment.
> This causes a crash if the function is called in such case.
Is the crash caused by the below assertion?
QualType Expr::findBoundMemberType(const Expr *expr) {
assert(expr->hasPlaceholderType(BuiltinType::BoundMember));
Comment at:
balazske added a comment.
Ping.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95244/new/
https://reviews.llvm.org/D95244
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
balazske added a comment.
Ping
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95244/new/
https://reviews.llvm.org/D95244
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
balazske added a comment.
Ping
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95244/new/
https://reviews.llvm.org/D95244
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
balazske added a comment.
ping
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95244/new/
https://reviews.llvm.org/D95244
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
martong added reviewers: rsmith, majnemer.
martong added subscribers: majnemer, rsmith.
martong added a comment.
Adding reviewers.
@rsmith as code owner,
@majnemer based on git blame of CallExpr::getCallReturnType.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
balazske added a comment.
I could add a test in ToolingTests, lit test is much more difficult to add
because the function is normally not called on "bad" code.
Comment at: clang/lib/AST/Expr.cpp:1403
+if (CalleeType.isNull())
+ return CreateDependentType();
+
balazske updated this revision to Diff 319261.
balazske added a comment.
Adding a test, fixing other problems revealed by the test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95244/new/
https://reviews.llvm.org/D95244
Files:
martong added a comment.
Hi Balázs,
Could you please create a lit test file which ignites the related crash?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95244/new/
https://reviews.llvm.org/D95244
___
balazske added inline comments.
Comment at: clang/lib/AST/Expr.cpp:1402
const FunctionType *FnType = CalleeType->castAs();
return FnType->getReturnType();
Here occurs the problem: If `CalleeType` is of type `BuiltinType::Overload` it
does not cast to
balazske added a comment.
Should add some responsible (for this code) reviewers.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95244/new/
https://reviews.llvm.org/D95244
___
cfe-commits mailing list
balazske created this revision.
Herald added subscribers: martong, gamesh411, Szelethus, dkrupp.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Type of `Callee` can be overloaded function type that was not handled.
This causes a
26 matches
Mail list logo
