steveire added inline comments.
================ Comment at: clang-tidy/ClangTidy.h:11 +// +// It should remain as stable as possible, as many out-of-tree checks exist. +//===----------------------------------------------------------------------===// ---------------- alexfh wrote: > sammccall wrote: > > steveire wrote: > > > Clang C++ code does not have any stability requirements. That's quite > > > well-established. > > > > > > This comment adds a new requirement of stability for this C++ API. You > > > should probably put a RFC on cfe-dev about it. This header is no more > > > public or stable than any other Clang header. > > > > > > > > > Clang C++ code does not have any stability requirements. That's quite > > > well-established. > > > > I was aiming for "stability is a design goal", not "this is a stable API > > and you may not break it". > > Happy to take a shot at rewording, maybe you have suggestions? > > > > > This comment adds a new requirement of stability for this C++ API. You > > > should probably put a RFC on cfe-dev about it. > > > > Stability here has always been an aim; I'm trying to document the current > > state of the world. > > > > e.g. in D15528 @alexfh wrote: > > > It's one of the goals of clang-tidy to provide an easy way to maintain > > > out-of-tree checks. > > > > > This header is no more public or stable than any other Clang header. > > I wish that were true - my current yakshave is reducing the need for > > `registerPPCallbacks` to make clang-tidy more flexible for use in a > > library. Removing it entirely would be great but having talked to some > > clang-tidy maintainers, out-of-tree checks are at least a factor here. > I tend to agree with Stephen re: lack of stability guarantees of Clang APIs. > Clang-tidy API on its own is not nearly enough to create a useful check, thus > any out-of-tree check will use a significant amount of Clang APIs. Though the > project aims at providing an easy way to maintain out-of-tree checks, there's > little benefit in clang-tidy APIs being more stable than the API surface of > Clang needed for an average out-of-tree check. Maintainers of any clang-based > out-of-tree code should be ready to update their code with every change of > the clang revision they use. Clang-tidy checks are not an exception. Putting > this documentation here may send a very wrong message and serve badly both to > the clang-tidy developers and to the folks who decide to depend on it. > > A softer version of this (something along the lines of "A large number of > out-of-tree checks depend on this API, so be careful when introducing > potentially breaking changes.") may work here, but the same note will be > needed on the whole AST, ASTMatchers, and Lex libraries in Clang (and many > other ones, I guess). I have a proposal coming up which involves a breaking change to AST_MATCHER* macros (many of which are out of tree), so I'm really just trying to make sure no backward compatibility concern opposes that. I still see no point in the comment, if you don't also put the same comment in ASTMatchers.h, each header in AST/ etc. The files have the same out-of-tree use, and no one of them deserves a comment more than the others. In fact, I had the idea just last week that I want to write a documentation page outlining compatibility guarantees in various user-facing parts of llvm/clang. It would be a single page containing the two sections about compatibility here: https://llvm.org/docs/DeveloperPolicy.html#ir-backwards-compatibility, C++ API stability, whether clang-query interpreter commands are stable, whether driver command lines and plumbing command lines are stable etc. AFAIK, that does not exist yet. Regarding C++ API compatibility something along the lines here (http://clang-developers.42468.n3.nabble.com/getLocStart-versus-getStartLoc-tp4061010p4061292.html) and here (http://clang-developers.42468.n3.nabble.com/API-Removal-Notice-4-weeks-getStartLoc-getLocStart-getLocEnd-td4061566.html) . A note like yours would belong on such a page too. I don't think it belongs in this header. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53936 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
