delcypher wrote: > Unfortunately, I think this should be held off until we have a bigger picture > for the future of `Sema`. I'll elaborate below. > > If you take a close look at the existing set of `Sema` parts, it's almost > entirely comprised of other languages or language extensions (from C and C++ > standpoint) and backends. They were considered natural enough to be split off > `Sema`, which, among other things, means that it's easy to explain their > place in the big picture, like I did in the previous sentence.
As noted above `-fbounds-safety` **is a C language extension** which makes it seem like it would fit nicely into the existing division of Sema into multiple objects and relevant source files. > I don't think this patch makes it easier to explain people where things are > in this codebase, because (I think) `counted_by` is a declaration attribute, > and we already have place for them. Having only one function also makes it a > very small part of `Sema`, which doesn't sound compelling. It doesn't right now because most of `-fbounds-safety` implementation isn't upstream yet. I simply moved what is **currently upstream**. As we (Apple) upstream more and more of the implementation it makes a lot sense to try to keep as much of it in its own source file to: * Create a clean separation between `-fbounds-safety` Sema checks and everything else. This will ultimately make exploring the code easier once a significant amount of our implementation has been upstreamed. * Avoid growing all the existing `Sema*.cpp` files unnecessarily. They are already huge and we don't want to make the problem worse when there's an easy option available to avoid that. > A bigger picture this fits in would be a compelling argument (like it was for > small backend parts), but to my knowledge there's none. The `counted_by` attribute is just one of several attributes that are part of the `-fbounds-safety` language extension which is documented here https://clang.llvm.org/docs/BoundsSafety.html. That's "bigger picture" for the feature, or did you mean something else by "bigger picture this fits in"? > Coming up with one and getting maintainers on board is certainly out of scope > of this PR, so don't feel obligated to do that. Hopefully I'll get back to > this whole topic later. > My end goal is to have "somewhere" to put all of the Sema code that support `-fbounds-safety` as we upstream it. I think it's very much preferable to have this code go into its own source file and header file for the reasons I gave above. We don't have to use the sub-class `SemaBase` mechanism that I've used here if this is really a problem. However, the mechanism does seem like a good fit for `-fbounds-safety` and would be a shame not to use it. https://github.com/llvm/llvm-project/pull/98954 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
