pmatos marked 6 inline comments as done.
pmatos added inline comments.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:6552-6555
+ // The call we get looks like
+ // CallExpr
+ // `- ImplicitCastExpr
+ // `- DeclRefExpr
----------------
tlively wrote:
> How do these parts correspond to the original source code?
If I understand your question correctly, this is just the semantic checks for
each of the builtins.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:6564
+ // Prepare FDecl type
+ QualType Pointee = Context.getFunctionType(Context.VoidTy, {}, {});
+ QualType Type = Context.getPointerType(Pointee);
----------------
aaron.ballman wrote:
> Why are you guaranteed this function returns void?
I think this question is not relevant anymore given the current code, right?
The wasm ref null builtin will always return the null funcref.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:6704-6710
+ // Therefore we need to change the types of the DeclRefExpr (stored in FDecl)
+ // and regenerate a straight up CallExpr on the modified FDecl.
+ // returning
+ // CallExpr
+ // `- FunctionDecl
+
+ // Prepare FDecl type
----------------
aaron.ballman wrote:
> These comments no longer seem to match the code -- nothing creates a
> `FunctionDecl` here.
Right - fixed.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:6708
+ // CallExpr
+ // `- FunctionDecl
+
----------------
erichkeane wrote:
> All those comments don't really seem relevant here, and are not really
> correct. The only thing SemaChecking should do is either modify its
> arguments (though you don't have any), or set its return type.
Right, these comments can be removed. I fixed these.
================
Comment at: clang/lib/Sema/SemaType.cpp:7340
+ attr::Kind NewAttrKind = A->getKind();
+ QualType Desugared = Type;
+ const auto *AT = dyn_cast<AttributedType>(Type);
----------------
erichkeane wrote:
> This name seems misleading... should you be desugaring 'Type' to store it in
> 'Desugared'?
>
> Also, I see that this value is never used, so why do the copy init? ALSO,
> why is it called Desugared, when it never seems to get a Desugared type?
Bad naming due to earlier code. I have simplified this code now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128440/new/
https://reviews.llvm.org/D128440
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
