[PATCH] D146089: [Sema] Fix null pointer dereference handleAlwaysInlineAttr.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/Sema/SemaStmtAttr.cpp:236
     const Decl *Decl = CallExpr->getCalleeDecl();
     if (Decl->hasAttr<AlwaysInlineAttr>() || Decl->hasAttr<FlattenAttr>())
       S.Diag(St->getBeginLoc(), diag::warn_function_stmt_attribute_precedence)
----------------
Same fix + a test probably needed here too.


================
Comment at: clang/test/Sema/attr-alwaysinline.cpp:32
+int foo(int x) {
+    if constexpr (D > 1)
+        [[clang::always_inline]] return foo<D-1>(x + 1);
----------------
Also, I note the 'if constexpr' branch is unnecessary to reproduce this.


================
Comment at: clang/test/Sema/attr-alwaysinline.cpp:36
+        return x;
+}
+
----------------
craig.topper wrote:
> erichkeane wrote:
> > craig.topper wrote:
> > > erichkeane wrote:
> > > > Can you add a test that shows that we warn on instantiation?  This shouldn't be a dependent declrefexpr when instantiated.
> > > > 
> > > > Additionally, this would make sure that we're properly propoagating `always_inline`.
> > > Should this warn
> > > 
> > > ```
> > > template<int D> [[gnu::noinline]]                                                
> > > int bar(int x) {                                                                 
> > >     if constexpr (D > 1)                                                         
> > >         [[clang::always_inline]] return bar<D-1>(x + 1);                         
> > >     else                                                                         
> > >         return x;                                                                
> > > }                                                                                
> > >                                                                                  
> > > int baz(int x) {                                                                 
> > >   return bar<5>(x);                                                              
> > > }  
> > > ```
> > Yes, I would expect that to warn.
> It looks like handleAlwaysInlineAttr only gets called once so it doesn't get called after instantiation.
Hmm... thats unfortunate.  That means we're perhaps not instantiating it correctly.  I'll take some time to poke around as I get a chacne.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146089