mgehre added a comment.
I now add the attributes to all redeclarations to make the logic a bit easier
to follow.
================
Comment at: clang/lib/Sema/SemaAttr.cpp:94
- Canonical->addAttr(::new (Context) Attribute(SourceRange{}, Context,
- /*DerefType*/ nullptr,
- /*Spelling=*/0));
+ Record->addAttr(::new (Context) Attribute(SourceRange{}, Context,
+ /*DerefType*/ nullptr,
----------------
xazax.hun wrote:
> Doesn't the attribute have a `CreateImplicit` static method? If so, we could
> use that :)
Nice, didn't know that!
================
Comment at: clang/test/SemaCXX/attr-gsl-owner-pointer-std.cpp:95
+// The iterator typedef is a DependentNameType.
+template <typename T>
----------------
gribozavr wrote:
> mgehre wrote:
> > gribozavr wrote:
> > > This test file is getting pretty long and subtle, with lots of things are
> > > being mixed into one file without isolation.
> > >
> > > WDYT about refactoring it into a unit test that uses AST matchers to
> > > verify attribute presence?
> > >
> > > See clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp for examples.
> > >
> > > Each test case would look approximately like this:
> > >
> > > ```
> > > EXPECT_TRUE(matches(
> > > "template class ...",
> > > classTemplateDecl(hasName("foo"), hasAttr(clang::attr::GslPointer)));
> > > ```
> > >
> > > Each test case would be isolated from other tests, each test would have a
> > > name (and optionally a comment) that will make it obvious what exactly is
> > > being tested.
> > >
> > > It would be also possible to verify things like "The iterator typedef is
> > > a DependentNameType" to ensure that we're testing exactly what we want to
> > > test.
> > I like the AST matcher approach! This test file is really hard to debug - I
> > usually copy a test to its own file for debugging.
> > Would you be okay with deferring the testing change to the next PR?
> Of course! Thanks!
I added the some tests for this particular change to a new unittest and will
migrate the rest of the tests later.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66179/new/
https://reviews.llvm.org/D66179
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
