[PATCH] D153469: [dataflow] Replace most BoolValue subclasses with references to Formula (and AtomicBoolValue => Atom and BoolValue => Formula where appropriate)

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added inline comments.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:532
+  void addToFlowCondition(const Formula &);
+  LLVM_DEPRECATED("Use Formula version instead", "")
   void addToFlowCondition(BoolValue &Val);
----------------
sammccall wrote:
> uabelho wrote:
> > sammccall wrote:
> > > uabelho wrote:
> > > > There are still uses of this method in-tree. I get the following error (and many more) when I compile trunk now with -Werror:
> > > > ```
> > > > ../../clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:148:9: error: 'addToFlowCondition' is deprecated: Use Formula version instead [-Werror,-Wdeprecated-declarations]
> > > >     Env.addToFlowCondition(*Val);
> > > >         ^
> > > > ../../clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:532:3: note: 'addToFlowCondition' has been explicitly marked deprecated here
> > > >   LLVM_DEPRECATED("Use Formula version instead", "")
> > > >   ^
> > > > ../include/llvm/Support/Compiler.h:155:50: note: expanded from macro 'LLVM_DEPRECATED'
> > > > #define LLVM_DEPRECATED(MSG, FIX) __attribute__((deprecated(MSG, FIX)))
> > > >                                                  ^
> > > > 1 error generated.
> > > > ```
> > > Sure, I plan to clean these up once this change sticks.
> > > 
> > > Do you think -Wdeprecated -Werror is a build config we should keep working at all times?
> > > This would mean you we can't use deprecation to migrate internal callers over multiple patches, essentially 
> > > 
> > > So building with -Wno-error=deprecated-declarations or so seems to make more sense to me, but I can't see anything written either way. (Nor buildbot mail)
> > There are bots failing now at least:
> >  https://lab.llvm.org/buildbot/#/builders/57
> >  https://lab.llvm.org/buildbot/#/builders/214
> > 
> > I *think* deprecation is mainly used to let downstream adapt? And when deprecating stuff in-tree code is usually fixed at once?
> > 
> Fair enough, thanks for the bot links
> 
> I don't think expanding the scope of the change for now is a good idea.
> 9d5cfed2443aa294e67bea694d5aab02c40fa278 replaces [[deprecated]] with a comment.
Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153469/new/

https://reviews.llvm.org/D153469