George wanted to use it from clang (or, at least can't use from anything from clang-tidy.) I was actually assuming the intended usage is from some tooling instead of the frontend itself, but I never checked with George. Personally I don't think ExprMutationAnalyzer should be used in anywhere other than toolings. I'm happy to move it around if current location is not desirable.
On Mon, Oct 1, 2018 at 2:50 PM Richard Smith <rich...@metafoo.co.uk> wrote: > This looks like the wrong fix to me, but I don't really know enough about > what's being done with ExprMutationAnalyzer to have an opinion on what the > right fix is. > > Shuai, what is the goal here? Why is this code being moved to Analysis/? > Do you intend to call it from the compiler frontend at some point? I can > see a code review for the move itself, but no discussion of a plan or > overall direction being followed here. Did I miss it? > > We have historically decided to not use the tooling interfaces > (ASTMatcher, ParentMap, etc) from the frontend, because they're generally a > poor fit for our goals (we aim for a largely-single-pass compilation with > good locality, and the AST matchers make different design choices). If you > want to change that, we'll need to discuss that decision. > > If the idea is to move this code into clang proper so that it can be used > by various different tooling clients, we'd need to discuss the right place > for it. Perhaps lib/Tooling/Analysis would make sense? > > On Mon, 1 Oct 2018 at 13:13, David Blaikie via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> I can't really reproduce this - when I try to build clang/llvm with >> LLVM_ENABLE_MODULES in CMake I'm still seeing an error I reported March on >> a cfe-dev thread - something to do with unique_ptr instantiations for >> MappedBlockStream in the PDB parsing code. >> >> So, I'm wondering what error you hit, Eric/where/how, etc... >> >> On Sun, Sep 30, 2018 at 10:23 AM Eric Fiselier <e...@efcs.ca> wrote: >> >>> +rsmith (actually this time) >>> >>> On Sun, Sep 30, 2018 at 12:09 PM Eric Fiselier <e...@efcs.ca> wrote: >>> >>>> +rsmith >>>> >>>> Hi All, >>>> >>>> Sorry, I'm not actually sure why this fix is correct.I stole both the >>>> fix and the comment from a similar one on L150 of the module map left by >>>> Richard Smith. >>>> >>>> /Eric >>>> >>>> On Tue, Sep 25, 2018 at 8:36 PM Shuai Wang <shuaiw...@google.com> >>>> wrote: >>>> >>>>> I'd like to understand this better as well, in particular what would >>>>> be a proper fix? >>>>> >>>>> On Tue, Sep 25, 2018 at 2:15 PM David Blaikie <dblai...@gmail.com> >>>>> wrote: >>>>> >>>>>> +Shuai Wang >>>>>> >>>>>> On Tue, Sep 25, 2018 at 2:14 PM David Blaikie <dblai...@gmail.com> >>>>>> wrote: >>>>>> >>>>>>> Hey Eric - thanks for the fix - but could you explain the issue here >>>>>>> in a bit more detail, as I'm a bit confused (& really interested in >>>>>>> understanding any layering problems in LLVM - and fixing them/making >>>>>>> sure >>>>>>> they're fixed/holding the line/etc) >>>>>>> >>>>>>> What do you mean by "pull all of the AST matchers library into >>>>>>> clang" - how does including a header ever add a link dependency? >>>>>>> >>>>>>> - Dave >>>>>>> >>>>>>> >>>>>>> On Sat, Sep 22, 2018 at 5:49 PM Eric Fiselier via cfe-commits < >>>>>>> cfe-commits@lists.llvm.org> wrote: >>>>>>> >>>>>>>> Author: ericwf >>>>>>>> Date: Sat Sep 22 17:48:05 2018 >>>>>>>> New Revision: 342827 >>>>>>>> >>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=342827&view=rev >>>>>>>> Log: >>>>>>>> Fix modules build with shared library. >>>>>>>> >>>>>>>> r341994 caused clangAnalysis to pull all of the AST matchers >>>>>>>> library into clang. Due to inline key functions in the headers, >>>>>>>> importing the AST matchers library gives a link dependency on the >>>>>>>> AST matchers (and thus the AST), which clang should not >>>>>>>> have. >>>>>>>> >>>>>>>> This patch works around the issues by excluding the offending >>>>>>>> libclangAnalysis header in the modulemap. >>>>>>>> >>>>>>>> Modified: >>>>>>>> cfe/trunk/include/clang/module.modulemap >>>>>>>> >>>>>>>> Modified: cfe/trunk/include/clang/module.modulemap >>>>>>>> URL: >>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/module.modulemap?rev=342827&r1=342826&r2=342827&view=diff >>>>>>>> >>>>>>>> ============================================================================== >>>>>>>> --- cfe/trunk/include/clang/module.modulemap (original) >>>>>>>> +++ cfe/trunk/include/clang/module.modulemap Sat Sep 22 17:48:05 >>>>>>>> 2018 >>>>>>>> @@ -5,6 +5,12 @@ module Clang_Analysis { >>>>>>>> textual header "Analysis/Analyses/ThreadSafetyOps.def" >>>>>>>> >>>>>>>> module * { export * } >>>>>>>> + >>>>>>>> + // FIXME: Exclude these headers to avoid pulling all of the AST >>>>>>>> matchers >>>>>>>> + // library into clang. Due to inline key functions in the >>>>>>>> headers, >>>>>>>> + // importing the AST matchers library gives a link dependency on >>>>>>>> the AST >>>>>>>> + // matchers (and thus the AST), which clang-format should not >>>>>>>> have. >>>>>>>> + exclude header "Analysis/Analyses/ExprMutationAnalyzer.h" >>>>>>>> } >>>>>>>> >>>>>>>> module Clang_AST { >>>>>>>> >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> cfe-commits mailing list >>>>>>>> cfe-commits@lists.llvm.org >>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>>>>>> >>>>>>> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits