hfinkel added inline comments.
================ Comment at: clang/lib/Sema/SemaChecking.cpp:4489 + // Alignment calculations can wrap around if it's greater than 2**29. + unsigned MaximumAlignment = 536870912; + if (I > MaximumAlignment) ---------------- jdoerfert wrote: > erichkeane wrote: > > jdoerfert wrote: > > > erichkeane wrote: > > > > I thought we had this stored somewhere else? We probably should have > > > > this be a constant somewhere in the frontend. I THINK I remember doing > > > > a review where I pulled this value into clang somewhere... > > > That was D72998, and I don't think Clang is the right place for this > > > constant. It is a property of the llvm alignment attribute and it should > > > live there. Thus, llvm/include/Attributes.h or some similar place. Can't > > > we "fix" the linker error by making it a constexpr global or are the > > > errors because of other file content? If the latter, we could go with a > > > llvm/include/magic_constants.h ;) > > The one I was thinking of was this one: https://reviews.llvm.org/D68824 > > > > I don't remember what we came up with on the linking issue. It would be > > really nice if it was just something included from LLVM, but I think SEMA > > typically doesn't include stuff from LLVM either. > I'm not too happy with the duplication of the constant but defining it once > in clang is certainly better than having it in N places. For OpenMP we look > into LLVM during SEMA and here there is an argument to be made that we should > as well. I imagine more cases would pop up over time. > > FWIW, if we allow to include LLVM headers, e.g., from IR or Frontend, we > could still have a wrapper in SEMA to get the information so it doesn't > expose the llvm:: namespace at the use sides (if that helps). > For OpenMP we look into LLVM during SEMA How do we do that? There's certainly an interesting philosophical issue around whether changes in LLVM should directly manifest as Clang behavioral changes, especially in -fsyntax-only. The answer to this question might be different for extensions vs. core language features (although alignment restrictions might implicate both). AFAIKT, historically , our answer has been to insist on separation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72996/new/ https://reviews.llvm.org/D72996 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
