erichkeane added a comment.
Explanations make sense to me, I'm generally in favor with the 1 concern.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:397
+ bool IsIFunc = isa<llvm::GlobalIFunc>(Alias);
llvm::Constant *Aliasee =
----------------
ibookstein wrote:
> erichkeane wrote:
> > What is the purpose of changing this from checking the declaration to the
> > IR? It seems that this is something we should be able to catch at the AST
> > level and not have to revert to IR checks like this.
> Inside `checkAliasedGlobal` I wanted to save on a parameter (`D`) since then
> I would just be passing it redundantly because the information is already
> available on the `Alias`. Here I wanted the check to be an exact copy of the
> check inside `checkAliasedGlobal` for consistency. I don't mind changing that
> at at all, I guess I just don't see why I should prefer one over the other :)
The CFE (particularly when doing checks, but even when generating code) should
be doing as much based on the AST rather than the IR, as it makes us a bit more
fragile/dependent on the form of the IR. So typcially we try to avoid
determining types/etc from IR.
Note we break this rule a bunch, but it is currently burning us on opaque-ptr
for example.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112868/new/
https://reviews.llvm.org/D112868
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
