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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to