Commit 21632b52 authored by David Van Cleve's avatar David Van Cleve Committed by Commit Bot

blink: Add absl::variant to the blink symbol allowlist

absl::variant was recently approved for general Chromium usage. This CL
adds it to the Blink symbol allowlist.

(I had a union I wanted to implement in the child CL and remembered
`variant` had recently been approved. I'm not familiar with the criteria
for permitting/forbidding symbols in Blink, so I wouldn't be too
surprised if it turns out we don't want `variant` in Blink.)

R=dcheng

Bug: None
Change-Id: I0885ef203472bcdb17481423d88eeb2add801766
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2442230
Commit-Queue: David Van Cleve <davidvc@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819703}
parent c776c87e
......@@ -508,6 +508,10 @@ _CONFIG = [
'base::mac::(CFToNSCast|NSToCFCast)',
'base::mac::Is(AtMost|AtLeast)?OS.+',
'base::(scoped_nsobject|ScopedCFTypeRef)',
# absl::variant and getters:
'absl::variant',
'absl::get_if',
],
'disallowed': [
('base::Bind(|Once|Repeating)',
......
......@@ -9,6 +9,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/ASTMatchers/ASTMatchersMacros.h"
using namespace clang::ast_matchers;
......@@ -42,7 +43,7 @@ class UniquePtrGarbageCollectedMatcher : public MatchFinder::MatchCallback {
match_finder.addDynamicMatcher(make_unique_matcher, this);
}
void run(const MatchFinder::MatchResult& result) {
void run(const MatchFinder::MatchResult& result) override {
auto* bad_use = result.Nodes.getNodeAs<clang::Expr>("bad");
auto* bad_function = result.Nodes.getNodeAs<clang::FunctionDecl>("badfunc");
auto* gc_type = result.Nodes.getNodeAs<clang::CXXRecordDecl>("gctype");
......@@ -72,7 +73,7 @@ class OptionalGarbageCollectedMatcher : public MatchFinder::MatchCallback {
match_finder.addDynamicMatcher(optional_construction, this);
}
void run(const MatchFinder::MatchResult& result) {
void run(const MatchFinder::MatchResult& result) override {
auto* bad_use = result.Nodes.getNodeAs<clang::Expr>("bad");
auto* optional = result.Nodes.getNodeAs<clang::CXXRecordDecl>("optional");
auto* gc_type = result.Nodes.getNodeAs<clang::CXXRecordDecl>("gctype");
......@@ -83,6 +84,60 @@ class OptionalGarbageCollectedMatcher : public MatchFinder::MatchCallback {
DiagnosticsReporter& diagnostics_;
};
// For the absl::variant checker, we need to match the inside of a variadic
// template class, which doesn't seem easy with the built-in matchers: define a
// custom matcher to go through the template parameter list.
AST_MATCHER_P(clang::TemplateArgument,
parameterPackHasAnyElement,
// Clang exports other instantiations of Matcher via
// using-declarations in public headers, e.g. `using TypeMatcher =
// Matcher<QualType>`.
//
// Once https://reviews.llvm.org/D89920, a Clang patch adding a
// similar alias for template arguments, lands, this can be
// changed to TemplateArgumentMatcher and won't need to use the
// internal namespace any longer.
clang::ast_matchers::internal::Matcher<clang::TemplateArgument>,
InnerMatcher) {
if (Node.getKind() != clang::TemplateArgument::Pack)
return false;
return llvm::any_of(Node.pack_elements(),
[&](const clang::TemplateArgument& Arg) {
return InnerMatcher.matches(Arg, Finder, Builder);
});
}
class VariantGarbageCollectedMatcher : public MatchFinder::MatchCallback {
public:
explicit VariantGarbageCollectedMatcher(DiagnosticsReporter& diagnostics)
: diagnostics_(diagnostics) {}
void Register(MatchFinder& match_finder) {
// Matches any constructed absl::variant where a template argument is
// known to refer to a garbage-collected type.
auto variant_construction =
cxxConstructExpr(
hasDeclaration(cxxConstructorDecl(
ofClass(classTemplateSpecializationDecl(
hasName("::absl::variant"),
hasAnyTemplateArgument(parameterPackHasAnyElement(
refersToType(GarbageCollectedType()))))
.bind("variant")))))
.bind("bad");
match_finder.addDynamicMatcher(variant_construction, this);
}
void run(const MatchFinder::MatchResult& result) override {
auto* bad_use = result.Nodes.getNodeAs<clang::Expr>("bad");
auto* variant = result.Nodes.getNodeAs<clang::CXXRecordDecl>("variant");
auto* gc_type = result.Nodes.getNodeAs<clang::CXXRecordDecl>("gctype");
diagnostics_.VariantUsedWithGC(bad_use, variant, gc_type);
}
private:
DiagnosticsReporter& diagnostics_;
};
} // namespace
void FindBadPatterns(clang::ASTContext& ast_context,
......@@ -95,5 +150,8 @@ void FindBadPatterns(clang::ASTContext& ast_context,
OptionalGarbageCollectedMatcher optional_gc(diagnostics);
optional_gc.Register(match_finder);
VariantGarbageCollectedMatcher variant_gc(diagnostics);
variant_gc.Register(match_finder);
match_finder.matchAST(ast_context);
}
......@@ -145,6 +145,10 @@ const char kOptionalUsedWithGC[] =
"[blink-gc] Disallowed construction of %0 found; %1 is a garbage-collected "
"type. optional cannot hold garbage-collected objects.";
const char kVariantUsedWithGC[] =
"[blink-gc] Disallowed construction of %0 found; %1 is a garbage-collected "
"type. absl::variant cannot hold garbage-collected objects.";
} // namespace
DiagnosticBuilder DiagnosticsReporter::ReportDiagnostic(
......@@ -246,6 +250,8 @@ DiagnosticsReporter::DiagnosticsReporter(
diagnostic_.getCustomDiagID(getErrorLevel(), kUniquePtrUsedWithGC);
diag_optional_used_with_gc_ =
diagnostic_.getCustomDiagID(getErrorLevel(), kOptionalUsedWithGC);
diag_variant_used_with_gc_ =
diagnostic_.getCustomDiagID(getErrorLevel(), kVariantUsedWithGC);
}
bool DiagnosticsReporter::hasErrorOccurred() const
......@@ -541,3 +547,11 @@ void DiagnosticsReporter::OptionalUsedWithGC(
ReportDiagnostic(expr->getBeginLoc(), diag_optional_used_with_gc_)
<< optional << gc_type << expr->getSourceRange();
}
void DiagnosticsReporter::VariantUsedWithGC(
const clang::Expr* expr,
const clang::CXXRecordDecl* variant,
const clang::CXXRecordDecl* gc_type) {
ReportDiagnostic(expr->getBeginLoc(), diag_variant_used_with_gc_)
<< variant << gc_type << expr->getSourceRange();
}
......@@ -82,6 +82,10 @@ class DiagnosticsReporter {
void OptionalUsedWithGC(const clang::Expr* expr,
const clang::CXXRecordDecl* optional,
const clang::CXXRecordDecl* gc_type);
void VariantUsedWithGC(const clang::Expr* expr,
const clang::CXXRecordDecl* variant,
const clang::CXXRecordDecl* gc_type);
private:
clang::DiagnosticBuilder ReportDiagnostic(
clang::SourceLocation location,
......@@ -139,6 +143,7 @@ class DiagnosticsReporter {
unsigned diag_unique_ptr_used_with_gc_;
unsigned diag_optional_used_with_gc_;
unsigned diag_variant_used_with_gc_;
};
#endif // TOOLS_BLINK_GC_PLUGIN_DIAGNOSTICS_REPORTER_H_
......@@ -174,6 +174,13 @@ class Optional {};
} // namespace base
namespace absl {
template <class... Ts>
class variant {};
} // namespace absl
namespace blink {
using namespace WTF;
......
......@@ -23,7 +23,7 @@ class DerivedHeapObject2 : public HeapObject {
./stack_allocated.h:44:3: warning: [blink-gc] Garbage collected class 'DerivedHeapObject2' is not permitted to override its new operator.
STACK_ALLOCATED();
^
./heap/stubs.h:188:5: note: expanded from macro 'STACK_ALLOCATED'
./heap/stubs.h:195:5: note: expanded from macro 'STACK_ALLOCATED'
__attribute__((annotate("blink_stack_allocated"))) \
^
In file included from stack_allocated.cpp:5:
......
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "variant_of_gced_type.h"
namespace blink {
void ForbidsVariantsOfGcedTypes() {
absl::variant<Base> not_ok;
(void)not_ok;
absl::variant<Base, Base> similarly_not_ok;
(void)similarly_not_ok;
absl::variant<int, Base> not_ok_either;
(void)not_ok_either;
absl::variant<int, Derived> ditto;
(void)ditto;
new absl::variant<Mixin>;
}
} // namespace blink
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef TOOLS_CLANG_BLINK_GC_PLUGIN_TESTS_VARIANT_OF_GCED_TYPE_H_
#define TOOLS_CLANG_BLINK_GC_PLUGIN_TESTS_VARIANT_OF_GCED_TYPE_H_
#include "heap/stubs.h"
namespace blink {
class Base : public GarbageCollected<Base> {
public:
virtual void Trace(Visitor*) const {}
};
class Derived : public Base {
public:
void Trace(Visitor* visitor) const override { Base::Trace(visitor); }
};
class Mixin : public GarbageCollectedMixin {
public:
void Trace(Visitor*) const {}
};
} // namespace blink
#endif // TOOLS_CLANG_BLINK_GC_PLUGIN_TESTS_VARIANT_OF_GCED_TYPE_H_
variant_of_gced_type.cpp:10:23: warning: [blink-gc] Disallowed construction of 'variant<blink::Base>' found; 'Base' is a garbage-collected type. absl::variant cannot hold garbage-collected objects.
absl::variant<Base> not_ok;
^~~~~~
variant_of_gced_type.cpp:13:29: warning: [blink-gc] Disallowed construction of 'variant<blink::Base, blink::Base>' found; 'Base' is a garbage-collected type. absl::variant cannot hold garbage-collected objects.
absl::variant<Base, Base> similarly_not_ok;
^~~~~~~~~~~~~~~~
variant_of_gced_type.cpp:16:28: warning: [blink-gc] Disallowed construction of 'variant<int, blink::Base>' found; 'Base' is a garbage-collected type. absl::variant cannot hold garbage-collected objects.
absl::variant<int, Base> not_ok_either;
^~~~~~~~~~~~~
variant_of_gced_type.cpp:19:31: warning: [blink-gc] Disallowed construction of 'variant<int, blink::Derived>' found; 'Derived' is a garbage-collected type. absl::variant cannot hold garbage-collected objects.
absl::variant<int, Derived> ditto;
^~~~~
variant_of_gced_type.cpp:22:7: warning: [blink-gc] Disallowed construction of 'variant<blink::Mixin>' found; 'Mixin' is a garbage-collected type. absl::variant cannot hold garbage-collected objects.
new absl::variant<Mixin>;
^~~~
5 warnings generated.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment