r.stahl added a comment. Thanks for the comments! All good points. Nice idea with the constructor, but that can probably happen in a follow up patch.
In D46421#1380619 <https://reviews.llvm.org/D46421#1380619>, @xazax.hun wrote: > I know you probably tired of me making you split all the patches but I think > if it is independent of CTU we should submit this fix as a separate commit. > This way we could be more focused having one commit doing one thing. No problem if I should do that. However I'm concerned whether this change is correct. Previously this was not required since all VarDecls were canonical. Not sure if this change was intended. I did some digging, but am not familiar enough with the code base to figure out what changed. Does anyone have an idea about this? But I agree that if it wasn't a deliberate change, we could canonicalize in the DeclRegion constructor - or VarRegion since only those are redeclarable. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D46421/new/ https://reviews.llvm.org/D46421 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
