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