Clang AST updates for more details #152372
Open
+109,418
−91,277
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-hlsl @llvm/pr-subscribers-backend-risc-v Author: Nicholas Reimer (sei-nreimer) ChangesAST output modifications primarily focused on JSON enhancements for the SEI Redemption project. Some of the key changes are:
Examples// recursive pointer resolution
int ** c;
// function pointer information
long (*foo)(int (*)(short));
// return type information
int runFunctionTestA( char a );
// refID usage example.
int a; // (not in image) first time encountering an int
int *b; // (not in image) first time encountering int *, but second time encountering int
int **c; // first time encountering int **, but second time encountering int * and third time int
Patch is 9.88 MiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/152372.diff 174 Files Affected:
diff --git a/clang/include/clang/AST/ASTNodeTraverser.h b/clang/include/clang/AST/ASTNodeTraverser.h
index 8ebabb2bde10d..b282f748a1426 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -11,6 +11,22 @@
// similar to RecursiveASTVisitor.
//
//===----------------------------------------------------------------------===//
+//
+// Modifications to this file by SEI staff are copyright Carnegie Mellon
+// University and contributed under the Apache License v2.0 with LLVM
+// Exceptions.
+//
+// SEI Contributions are made with funding sand support from the Department of
+// Defense under Contract No. FA8702-15-D-0002 with Carnegie Mellon University
+// for the operation of the Software Engineering Institute, a federally funded
+// research and development center.
+//
+// The view, opinions, and/or findings contained in this material are those of
+// the author(s) and should not be construed as an official Government position,
+// policy, or decision, unless designated by other documentation.
+// DM24-0194
+//
+//===----------------------------------------------------------------------===//
#ifndef LLVM_CLANG_AST_ASTNODETRAVERSER_H
#define LLVM_CLANG_AST_ASTNODETRAVERSER_H
@@ -177,14 +193,34 @@ class ASTNodeTraverser
if (!SQT.Quals.hasQualifiers())
return Visit(SQT.Ty);
- getNodeDelegate().AddChild([=] {
+ // SEI: changed from default label to "qualTypeDetail"
+ getNodeDelegate().AddChild("qualTypeDetail", [this, T] {
getNodeDelegate().Visit(T);
Visit(T.split().Ty);
});
+
+ // SEI function pointer support. this gets called whenever the three
+ // conditions are met:
+ // 1. the function pointer is not typedef'd
+ // 2. after Visit(VarDecl *) gets called
+ // 3. if VarDecl determines this is a function pointer
+ if (T->isFunctionPointerType()) {
+ // create as a child node to this type
+ getNodeDelegate().AddChild(
+ [=] { getNodeDelegate().Visit(T->getPointeeType()); });
+ }
+
+ // SEI: traverse PointerType information
+ if (T->isPointerType())
+ Visit(T->getPointeeType());
}
+ // SEI: traverse ReturnType information
+ void VisitReturnType(QualType T) { getNodeDelegate().VisitReturnType(T); }
+
void Visit(const Type *T) {
- getNodeDelegate().AddChild([=] {
+ // SEI: renamed this from default label
+ getNodeDelegate().AddChild("typeDetails", [this, T] {
getNodeDelegate().Visit(T);
if (!T)
return;
@@ -209,7 +245,8 @@ class ASTNodeTraverser
}
void Visit(const Attr *A) {
- getNodeDelegate().AddChild([=] {
+ // SEI: renamed from default label
+ getNodeDelegate().AddChild("attrDetails", [this, A] {
getNodeDelegate().Visit(A);
ConstAttrVisitor<Derived>::Visit(A);
});
@@ -416,8 +453,20 @@ class ASTNodeTraverser
Visit(T->getSizeExpr());
}
void VisitVectorType(const VectorType *T) { Visit(T->getElementType()); }
- void VisitFunctionType(const FunctionType *T) { Visit(T->getReturnType()); }
+ void VisitFunctionType(const FunctionType *T) {
+
+ Visit(T->getReturnType());
+
+ // SEI: add functionDetails, incl. return type
+ getNodeDelegate().AddChild("functionDetails", [this, T] {
+ getNodeDelegate().VisitFunctionType(T);
+ getNodeDelegate().VisitReturnType(T->getReturnType());
+ });
+ }
+
void VisitFunctionProtoType(const FunctionProtoType *T) {
+
+ // SEI: visit the function type. this will force the return type info too.
VisitFunctionType(T);
for (const QualType &PT : T->getParamTypes())
Visit(PT);
@@ -585,6 +634,12 @@ class ASTNodeTraverser
Visit(TSI->getTypeLoc());
if (D->hasInit())
Visit(D->getInit());
+
+ // SEI: if this is a function pointer, then we need to get the
+ // FunctionProtoType and then make add'l visits. if the FP is typedef'd,
+ // then this behavior occurs for us outside of Visit(VarDecl *)
+ //getNodeDelegate().Visit(D->getType(), true);
+ Visit(D->getType());
}
void VisitDecompositionDecl(const DecompositionDecl *D) {
diff --git a/clang/include/clang/AST/JSONNodeDumper.h b/clang/include/clang/AST/JSONNodeDumper.h
index 570662b58ccf0..f20303f0509f0 100644
--- a/clang/include/clang/AST/JSONNodeDumper.h
+++ b/clang/include/clang/AST/JSONNodeDumper.h
@@ -10,6 +10,22 @@
// a JSON.
//
//===----------------------------------------------------------------------===//
+//
+// Modifications to this file by SEI staff are copyright Carnegie Mellon
+// University and contributed under the Apache License v2.0 with LLVM
+// Exceptions.
+//
+// SEI Contributions are made with funding sand support from the Department of
+// Defense under Contract No. FA8702-15-D-0002 with Carnegie Mellon University
+// for the operation of the Software Engineering Institute, a federally funded
+// research and development center.
+//
+// The view, opinions, and/or findings contained in this material are those of
+// the author(s) and should not be construed as an official Government position,
+// policy, or decision, unless designated by other documentation.
+// DM24-0194
+//
+//===----------------------------------------------------------------------===//
#ifndef LLVM_CLANG_AST_JSONNODEDUMPER_H
#define LLVM_CLANG_AST_JSONNODEDUMPER_H
@@ -26,6 +42,9 @@
#include "clang/AST/Type.h"
#include "llvm/Support/JSON.h"
+// SEI: added for caching addresses of certain visited nodes
+#include <unordered_set>
+
namespace clang {
class APValue;
@@ -111,8 +130,8 @@ class NodeStreamer {
// Dumps AST nodes in JSON format. There is no implied stability for the
// content or format of the dump between major releases of Clang, other than it
// being valid JSON output. Further, there is no requirement that the
-// information dumped is a complete representation of the AST, only that the
-// information presented is correct.
+// information dumped be a complete representation of the AST, only that the
+// information presented be correct.
class JSONNodeDumper
: public ConstAttrVisitor<JSONNodeDumper>,
public comments::ConstCommentVisitor<JSONNodeDumper, void,
@@ -132,6 +151,9 @@ class JSONNodeDumper
StringRef LastLocFilename, LastLocPresumedFilename;
unsigned LastLocLine, LastLocPresumedLine;
+ // SEI: caches addresses for QualType nodes that are duplicates
+ std::unordered_set<void *> AddressCache;
+
using InnerAttrVisitor = ConstAttrVisitor<JSONNodeDumper>;
using InnerCommentVisitor =
comments::ConstCommentVisitor<JSONNodeDumper, void,
@@ -184,6 +206,18 @@ class JSONNodeDumper
StringRef getCommentCommandName(unsigned CommandID) const;
+ /// SEI: simple cacher for addresses of nodes to reduce
+ /// bloat caused by SEI changes
+ /// Return True if it's already cached, otherwise false
+ bool cacheAddress(void *p) {
+ if (AddressCache.find(p) == AddressCache.end()) {
+ AddressCache.insert(p);
+ return false;
+ }
+
+ return true;
+ }
+
public:
JSONNodeDumper(raw_ostream &OS, const SourceManager &SrcMgr, ASTContext &Ctx,
const PrintingPolicy &PrintPolicy,
@@ -196,6 +230,17 @@ class JSONNodeDumper
void Visit(const Stmt *Node);
void Visit(const Type *T);
void Visit(QualType T);
+
+ // SEI: forwarding function to Visit(QualType T); so that it's applied here
+ // from a VarDecl call
+ void Visit(QualType T, bool isFromDecl);
+
+ // SEI: get specific details from the qual type
+ void VisitQualTypeDetails(QualType T);
+
+ // SEI: traverse ReturnType information
+ void VisitReturnType(QualType T);
+
void Visit(const Decl *D);
void Visit(TypeLoc TL);
diff --git a/clang/include/clang/AST/TextNodeDumper.h b/clang/include/clang/AST/TextNodeDumper.h
index 1917a8ac29f05..1340fdb4acb51 100644
--- a/clang/include/clang/AST/TextNodeDumper.h
+++ b/clang/include/clang/AST/TextNodeDumper.h
@@ -9,6 +9,22 @@
// This file implements AST dumping of components of individual AST nodes.
//
//===----------------------------------------------------------------------===//
+//
+// Modifications to this file by SEI staff are copyright Carnegie Mellon
+// University and contributed under the Apache License v2.0 with LLVM
+// Exceptions.
+//
+// SEI Contributions are made with funding sand support from the Department of
+// Defense under Contract No. FA8702-15-D-0002 with Carnegie Mellon University
+// for the operation of the Software Engineering Institute, a federally funded
+// research and development center.
+//
+// The view, opinions, and/or findings contained in this material are those of
+// the author(s) and should not be construed as an official Government position,
+// policy, or decision, unless designated by other documentation.
+// DM24-0194
+//
+//===----------------------------------------------------------------------===//
#ifndef LLVM_CLANG_AST_TEXTNODEDUMPER_H
#define LLVM_CLANG_AST_TEXTNODEDUMPER_H
@@ -181,8 +197,17 @@ class TextNodeDumper
void Visit(QualType T);
+ // SEI: added to prevent this from being added whenever it comes from VarDecl
+ void Visit(QualType T, bool isFromDecl);
+
void Visit(TypeLoc);
+ // SEI: added support for getting ReturnType information
+ void VisitReturnType(QualType T);
+
+ // SEI: added support for more QT details. it's a passthrough for this class
+ void VisitQualTypeDetails(QualType T) {}
+
void Visit(const Decl *D);
void Visit(const CXXCtorInitializer *Init);
diff --git a/clang/lib/AST/JSONNodeDumper.cpp b/clang/lib/AST/JSONNodeDumper.cpp
index 64ddb1e739347..11cd21e844dba 100644
--- a/clang/lib/AST/JSONNodeDumper.cpp
+++ b/clang/lib/AST/JSONNodeDumper.cpp
@@ -70,6 +70,19 @@ void JSONNodeDumper::Visit(const Stmt *S) {
}
void JSONNodeDumper::Visit(const Type *T) {
+ // SEI: ensure FPTs are debloated. this can be expanded to ALL types, if
+ // desired
+ if (T && cacheAddress((void *)T)) {
+ // add this as a child to know that it's a Type
+ AddChild("typeDetails",
+ [=] { JOS.attribute("refId", createPointerRepresentation(T)); });
+
+ InnerTypeVisitor::Visit(T);
+ // SEI
+ VisitQualTypeDetails(T->getCanonicalTypeInternal());
+ return;
+ }
+
JOS.attribute("id", createPointerRepresentation(T));
if (!T)
@@ -86,15 +99,50 @@ void JSONNodeDumper::Visit(const Type *T) {
T->containsUnexpandedParameterPack());
attributeOnlyIfTrue("isImported", T->isFromAST());
InnerTypeVisitor::Visit(T);
+ // SEI
+ VisitQualTypeDetails(T->getCanonicalTypeInternal());
}
void JSONNodeDumper::Visit(QualType T) {
- JOS.attribute("id", createPointerRepresentation(T.getAsOpaquePtr()));
- JOS.attribute("kind", "QualType");
- JOS.attribute("type", createQualType(T));
- JOS.attribute("qualifiers", T.split().Quals.getAsString());
+
+ // SEI: used AddChild to prevent qualType from being part added to a list
+ // JOS.attributeArray("qualTypes", [=] {
+
+ // SEI: force qualType into its own block, otherwise multiple Visits
+ // create a bunch of siblings, which is invalid JSON
+ JOS.attributeBegin("qualType");
+ JOS.objectBegin();
+
+ // SEI: cache visited addresses and add only its refId
+ // instead of the kind, type, quals, but leave the qual type details
+ // because those can differ among IDs
+ if (cacheAddress(T.getAsOpaquePtr())) {
+ JOS.attribute("refId", createPointerRepresentation(T.getAsOpaquePtr()));
+ } else {
+ JOS.attribute("id", createPointerRepresentation(T.getAsOpaquePtr()));
+ JOS.attribute("kind", "QualType");
+ JOS.attribute("type", createQualType(T));
+ JOS.attribute("qualifiers", T.split().Quals.getAsString());
+ }
+
+ // SEI: get add'l info required for redemption analysis
+ // the qual type details differ even among cached references
+ VisitQualTypeDetails(T);
+
+ // SEI: if this is a pointer type, then recursively call ourselves
+ // until it's not
+ if (T->isPointerType())
+ Visit(T->getPointeeType());
+
+ JOS.objectEnd();
+ JOS.attributeEnd();
+ //} );
}
+// SEI: added this as a forwarding function so that the TextNodeDumper
+// SEI: doesn't have this applied when coming from a VarDecl
+void JSONNodeDumper::Visit(QualType T, bool isFromDecl) { Visit(T); }
+
void JSONNodeDumper::Visit(TypeLoc TL) {
if (TL.isNull())
return;
@@ -110,6 +158,64 @@ void JSONNodeDumper::Visit(TypeLoc TL) {
[TL, this] { writeSourceRange(TL.getSourceRange()); });
}
+void JSONNodeDumper::VisitQualTypeDetails(QualType T) {
+ // SEI: get more detailed info on type. this info is not transferrable
+ // with the refId, so this must be called on every type even if that type
+ // has been cached
+ JOS.attributeBegin("qualDetails");
+ JOS.arrayBegin();
+
+ auto CT = T->getCanonicalTypeInternal();
+
+ if (CT->isStructureType())
+ JOS.value("struct");
+
+ if (CT->isNullPtrType())
+ JOS.value("null");
+ if (CT->isUndeducedType())
+ JOS.value("undeduced");
+
+ if (CT->isPointerType())
+ JOS.value("ptr");
+ if (CT->isVoidType())
+ JOS.value("void");
+
+ if (CT->isSignedIntegerType())
+ JOS.value("signed");
+ if (CT->isUnsignedIntegerType())
+ JOS.value("unsigned");
+ if (CT->isIntegerType())
+ JOS.value("integer");
+ if (CT->isFloatingType())
+ JOS.value("fpp");
+ if (CT->isEnumeralType())
+ JOS.value("enum");
+ if (CT->isUnionType())
+ JOS.value("union");
+ if (CT->isFunctionPointerType())
+ JOS.value("func_ptr");
+ if (CT->isTypedefNameType())
+ JOS.value("type_def");
+ if (CT->isArrayType())
+ JOS.value("array");
+
+ JOS.arrayEnd();
+ JOS.attributeEnd();
+}
+
+// SEI: capture the return info in a nested JSON block
+void JSONNodeDumper::VisitReturnType(QualType T) {
+ // using this function allows us to easily wrap just the returnType
+ // section into its own JSON block. if we do this in ASTNodeTraverser,
+ // then the TextNodeDumper works as expected but the JSONNodeDumper
+ // rolls all siblings into the returnType node with those siblings as child
+ // nodes
+
+ JOS.attributeObject("returnTypeDetail", [=] { Visit(T); });
+
+ // Visit(T);
+}
+
void JSONNodeDumper::Visit(const Decl *D) {
JOS.attribute("id", createPointerRepresentation(D));
@@ -174,6 +280,14 @@ void JSONNodeDumper::Visit(const TemplateArgument &TA, SourceRange R,
void JSONNodeDumper::Visit(const CXXCtorInitializer *Init) {
JOS.attribute("kind", "CXXCtorInitializer");
+
+ // SEI: added id for
+ JOS.attribute("id", createPointerRepresentation(Init));
+
+ // SEI: added range for CXXCtorInitializers
+ JOS.attributeObject(
+ "range", [Init, this] { writeSourceRange(Init->getSourceRange()); });
+
if (Init->isAnyMemberInitializer())
JOS.attribute("anyInit", createBareDeclRef(Init->getAnyMember()));
else if (Init->isBaseInitializer())
@@ -957,6 +1071,10 @@ void JSONNodeDumper::VisitFieldDecl(const FieldDecl *FD) {
attributeOnlyIfTrue("modulePrivate", FD->isModulePrivate());
attributeOnlyIfTrue("isBitfield", FD->isBitField());
attributeOnlyIfTrue("hasInClassInitializer", FD->hasInClassInitializer());
+
+ // SEI: had to add this in b/c FieldDecls do not seem to call
+ // Visit(QualType)
+ Visit(FD->getType());
}
void JSONNodeDumper::VisitFunctionDecl(const FunctionDecl *FD) {
@@ -1350,6 +1468,8 @@ void JSONNodeDumper::VisitDeclRefExpr(const DeclRefExpr *DRE) {
case NOUR_Discarded: JOS.attribute("nonOdrUseReason", "discarded"); break;
}
attributeOnlyIfTrue("isImmediateEscalating", DRE->isImmediateEscalating());
+ // SEI: this doesn't call VisitNamedDecl, so we force it
+ Visit(DRE->getType());
}
void JSONNodeDumper::VisitSYCLUniqueStableNameExpr(
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index 6b524cfcd2d71..d295faa7f3442 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -242,6 +242,21 @@ void TextNodeDumper::Visit(QualType T) {
OS << " " << T.split().Quals.getAsString();
}
+// SEI: ignore this from being called whenever it's from a VarDecl
+// SEI: so we don't interfere with the normal text output
+void TextNodeDumper::Visit(QualType T, bool isFromDecl) {
+ if (!isFromDecl)
+ Visit(T);
+}
+
+void TextNodeDumper::VisitReturnType(QualType T) {
+ OS << "ReturnType";
+ dumpPointer(T.getAsOpaquePtr());
+ OS << " ";
+ dumpBareType(T, false);
+ OS << " " << T.split().Quals.getAsString();
+}
+
void TextNodeDumper::Visit(TypeLoc TL) {
if (!TL) {
ColorScope Color(OS, ShowColors, NullColor);
diff --git a/clang/test/AST/ByteCode/builtin-constant-p.cpp b/clang/test/AST/ByteCode/builtin-constant-p.cpp
index 315a907949c34..9f5521590833d 100644
--- a/clang/test/AST/ByteCode/builtin-constant-p.cpp
+++ b/clang/test/AST/ByteCode/builtin-constant-p.cpp
@@ -140,11 +140,3 @@ void test17(void) {
F("string literal" + 1); // both-warning {{adding}} \
// both-note {{use array indexing}}
}
-
-/// FIXME
-static void foo(int i) __attribute__((__diagnose_if__(!__builtin_constant_p(i), "not constant", "error"))) // expected-note {{from}}
-{
-}
-static void bar(int i) {
- foo(15); // expected-error {{not constant}}
-}
diff --git a/clang/test/AST/ByteCode/complex.cpp b/clang/test/AST/ByteCode/complex.cpp
index be10b3cfa53da..10ca82fbdc485 100644
--- a/clang/test/AST/ByteCode/complex.cpp
+++ b/clang/test/AST/ByteCode/complex.cpp
@@ -409,4 +409,4 @@ namespace ComplexConstexpr {
static_assert(__real test6 == 5, "");
static_assert(__imag test6 == 6, "");
static_assert(&__imag test6 == &__real test6 + 1, "");
-}
+}
\ No newline at end of file
diff --git a/clang/test/AST/ByteCode/const-eval.c b/clang/test/AST/ByteCode/const-eval.c
index 3e228226ac8c1..eab14c08ec809 100644
--- a/clang/test/AST/ByteCode/const-eval.c
+++ b/clang/test/AST/ByteCode/const-eval.c
@@ -180,9 +180,6 @@ typedef __INTPTR_TYPE__ intptr_t;
const intptr_t A = (intptr_t)(((int*) 0) + 1);
const intptr_t B = (intptr_t)(((char*)0) + 3);
_Static_assert(A > B, "");
-int * GH149500_p = &(*(int *)0x400);
-static const void *GH149500_q = &(*(const struct sysrq_key_op *)0);
-
#else
#error :(
#endif
diff --git a/clang/test/AST/ByteCode/cxx23.cpp b/clang/test/AST/ByteCode/cxx23.cpp
index 45dd4f528aefb..2856b872d44ab 100644
--- a/clang/test/AST/ByteCode/cxx23.cpp
+++ b/clang/test/AST/ByteCode/cxx23.cpp
@@ -369,26 +369,7 @@ namespace NestedUnions {
return true;
}
static_assert(test_nested());
-}
-
-namespace UnionMemberCallDiags {
- struct A { int n; };
- struct B { A a; };
- constexpr A a = (A() = B().a);
- union C {
- int n;
- A a;
- };
- constexpr bool g() {
- C c = {.n = 1};
- c.a.operator=(B{2}.a); // all-note {{member call on member 'a' of union with active member 'n' is not allowed in a constant expression}}
- return c.a.n == 2;
- }
- static_assert(g()); // all-error {{not an integral constant expression}} \
- // all-note {{in call to}}
}
-
-
#endif
diff --git a/clang/test/AST/ByteCode/placement-new.cpp b/clang/test/AST/ByteCode/placement-new.cpp
index b587cd6eaf89c..670def2d5870e 100644
--- a/clang/test/AST/ByteCode/placement-new.cpp
+++ b/clang/test/AST/ByteCode/placement-new.cpp
@@ -486,11 +486,3 @@ namespace bitcast {
}
static_assert(foo() == 0);
}
-
-constexpr int modify_const_variable() {
- const int a = 10;
- new ((int *)&a) int(12); // both-note {{modification of object of const-qualified type 'const int' is not allowed in a constant expression}}
- return a;
-}
-static_assert(modify_const_variable()); // both-error {{not an integral constant expression}} \
- // both-note {{in call to}}
diff --git a/clang/test/AST/ByteCode/records.cpp b/clang/test/AST/ByteCode/records.cpp
index 5ca3e2d12e2df..8925f2a2d0b67 100644
--- a/clang/test/AST/ByteCode/records.cpp
+++ b/clang/test/AST/ByteCode/records.cpp
@@ -1839,4 +1839,4 @@ namespace DiamondDowncast {
constexpr Top &top1 = (Middle1&)bottom;
constexpr Middle2 &fail = (Middle2&)top1; // both-error {{must be initialized by a constant expression}} \
// both-note {{cannot cast object of dynamic type 'const Bottom' to type 'Middle2'}}
-}
+}
\ No newline at end of file
diff --git a/clang/test/AST/HLSL/ByteAddressBuffers-AST.hlsl b/clang/test/AST/HLSL/ByteAddressBuffers-AST.hlsl
index f85194496942b..74fcfa6ac6129 100644
--- a/clang/test/AST/HLSL/ByteAddressBuff...
[truncated]
|
|
@llvm/pr-subscribers-clang Author: Nicholas Reimer (sei-nreimer) ChangesAST output modifications primarily focused on JSON enhancements for the SEI Redemption project. Some of the key changes are:
Examples// recursive pointer resolution
int ** c;
// function pointer information
long (*foo)(int (*)(short));
// return type information
int runFunctionTestA( char a );
// refID usage example.
int a; // (not in image) first time encountering an int
int *b; // (not in image) first time encountering int *, but second time encountering int
int **c; // first time encountering int **, but second time encountering int * and third time int
Patch is 9.88 MiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/152372.diff 174 Files Affected:
diff --git a/clang/include/clang/AST/ASTNodeTraverser.h b/clang/include/clang/AST/ASTNodeTraverser.h
index 8ebabb2bde10d..b282f748a1426 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -11,6 +11,22 @@
// similar to RecursiveASTVisitor.
//
//===----------------------------------------------------------------------===//
+//
+// Modifications to this file by SEI staff are copyright Carnegie Mellon
+// University and contributed under the Apache License v2.0 with LLVM
+// Exceptions.
+//
+// SEI Contributions are made with funding sand support from the Department of
+// Defense under Contract No. FA8702-15-D-0002 with Carnegie Mellon University
+// for the operation of the Software Engineering Institute, a federally funded
+// research and development center.
+//
+// The view, opinions, and/or findings contained in this material are those of
+// the author(s) and should not be construed as an official Government position,
+// policy, or decision, unless designated by other documentation.
+// DM24-0194
+//
+//===----------------------------------------------------------------------===//
#ifndef LLVM_CLANG_AST_ASTNODETRAVERSER_H
#define LLVM_CLANG_AST_ASTNODETRAVERSER_H
@@ -177,14 +193,34 @@ class ASTNodeTraverser
if (!SQT.Quals.hasQualifiers())
return Visit(SQT.Ty);
- getNodeDelegate().AddChild([=] {
+ // SEI: changed from default label to "qualTypeDetail"
+ getNodeDelegate().AddChild("qualTypeDetail", [this, T] {
getNodeDelegate().Visit(T);
Visit(T.split().Ty);
});
+
+ // SEI function pointer support. this gets called whenever the three
+ // conditions are met:
+ // 1. the function pointer is not typedef'd
+ // 2. after Visit(VarDecl *) gets called
+ // 3. if VarDecl determines this is a function pointer
+ if (T->isFunctionPointerType()) {
+ // create as a child node to this type
+ getNodeDelegate().AddChild(
+ [=] { getNodeDelegate().Visit(T->getPointeeType()); });
+ }
+
+ // SEI: traverse PointerType information
+ if (T->isPointerType())
+ Visit(T->getPointeeType());
}
+ // SEI: traverse ReturnType information
+ void VisitReturnType(QualType T) { getNodeDelegate().VisitReturnType(T); }
+
void Visit(const Type *T) {
- getNodeDelegate().AddChild([=] {
+ // SEI: renamed this from default label
+ getNodeDelegate().AddChild("typeDetails", [this, T] {
getNodeDelegate().Visit(T);
if (!T)
return;
@@ -209,7 +245,8 @@ class ASTNodeTraverser
}
void Visit(const Attr *A) {
- getNodeDelegate().AddChild([=] {
+ // SEI: renamed from default label
+ getNodeDelegate().AddChild("attrDetails", [this, A] {
getNodeDelegate().Visit(A);
ConstAttrVisitor<Derived>::Visit(A);
});
@@ -416,8 +453,20 @@ class ASTNodeTraverser
Visit(T->getSizeExpr());
}
void VisitVectorType(const VectorType *T) { Visit(T->getElementType()); }
- void VisitFunctionType(const FunctionType *T) { Visit(T->getReturnType()); }
+ void VisitFunctionType(const FunctionType *T) {
+
+ Visit(T->getReturnType());
+
+ // SEI: add functionDetails, incl. return type
+ getNodeDelegate().AddChild("functionDetails", [this, T] {
+ getNodeDelegate().VisitFunctionType(T);
+ getNodeDelegate().VisitReturnType(T->getReturnType());
+ });
+ }
+
void VisitFunctionProtoType(const FunctionProtoType *T) {
+
+ // SEI: visit the function type. this will force the return type info too.
VisitFunctionType(T);
for (const QualType &PT : T->getParamTypes())
Visit(PT);
@@ -585,6 +634,12 @@ class ASTNodeTraverser
Visit(TSI->getTypeLoc());
if (D->hasInit())
Visit(D->getInit());
+
+ // SEI: if this is a function pointer, then we need to get the
+ // FunctionProtoType and then make add'l visits. if the FP is typedef'd,
+ // then this behavior occurs for us outside of Visit(VarDecl *)
+ //getNodeDelegate().Visit(D->getType(), true);
+ Visit(D->getType());
}
void VisitDecompositionDecl(const DecompositionDecl *D) {
diff --git a/clang/include/clang/AST/JSONNodeDumper.h b/clang/include/clang/AST/JSONNodeDumper.h
index 570662b58ccf0..f20303f0509f0 100644
--- a/clang/include/clang/AST/JSONNodeDumper.h
+++ b/clang/include/clang/AST/JSONNodeDumper.h
@@ -10,6 +10,22 @@
// a JSON.
//
//===----------------------------------------------------------------------===//
+//
+// Modifications to this file by SEI staff are copyright Carnegie Mellon
+// University and contributed under the Apache License v2.0 with LLVM
+// Exceptions.
+//
+// SEI Contributions are made with funding sand support from the Department of
+// Defense under Contract No. FA8702-15-D-0002 with Carnegie Mellon University
+// for the operation of the Software Engineering Institute, a federally funded
+// research and development center.
+//
+// The view, opinions, and/or findings contained in this material are those of
+// the author(s) and should not be construed as an official Government position,
+// policy, or decision, unless designated by other documentation.
+// DM24-0194
+//
+//===----------------------------------------------------------------------===//
#ifndef LLVM_CLANG_AST_JSONNODEDUMPER_H
#define LLVM_CLANG_AST_JSONNODEDUMPER_H
@@ -26,6 +42,9 @@
#include "clang/AST/Type.h"
#include "llvm/Support/JSON.h"
+// SEI: added for caching addresses of certain visited nodes
+#include <unordered_set>
+
namespace clang {
class APValue;
@@ -111,8 +130,8 @@ class NodeStreamer {
// Dumps AST nodes in JSON format. There is no implied stability for the
// content or format of the dump between major releases of Clang, other than it
// being valid JSON output. Further, there is no requirement that the
-// information dumped is a complete representation of the AST, only that the
-// information presented is correct.
+// information dumped be a complete representation of the AST, only that the
+// information presented be correct.
class JSONNodeDumper
: public ConstAttrVisitor<JSONNodeDumper>,
public comments::ConstCommentVisitor<JSONNodeDumper, void,
@@ -132,6 +151,9 @@ class JSONNodeDumper
StringRef LastLocFilename, LastLocPresumedFilename;
unsigned LastLocLine, LastLocPresumedLine;
+ // SEI: caches addresses for QualType nodes that are duplicates
+ std::unordered_set<void *> AddressCache;
+
using InnerAttrVisitor = ConstAttrVisitor<JSONNodeDumper>;
using InnerCommentVisitor =
comments::ConstCommentVisitor<JSONNodeDumper, void,
@@ -184,6 +206,18 @@ class JSONNodeDumper
StringRef getCommentCommandName(unsigned CommandID) const;
+ /// SEI: simple cacher for addresses of nodes to reduce
+ /// bloat caused by SEI changes
+ /// Return True if it's already cached, otherwise false
+ bool cacheAddress(void *p) {
+ if (AddressCache.find(p) == AddressCache.end()) {
+ AddressCache.insert(p);
+ return false;
+ }
+
+ return true;
+ }
+
public:
JSONNodeDumper(raw_ostream &OS, const SourceManager &SrcMgr, ASTContext &Ctx,
const PrintingPolicy &PrintPolicy,
@@ -196,6 +230,17 @@ class JSONNodeDumper
void Visit(const Stmt *Node);
void Visit(const Type *T);
void Visit(QualType T);
+
+ // SEI: forwarding function to Visit(QualType T); so that it's applied here
+ // from a VarDecl call
+ void Visit(QualType T, bool isFromDecl);
+
+ // SEI: get specific details from the qual type
+ void VisitQualTypeDetails(QualType T);
+
+ // SEI: traverse ReturnType information
+ void VisitReturnType(QualType T);
+
void Visit(const Decl *D);
void Visit(TypeLoc TL);
diff --git a/clang/include/clang/AST/TextNodeDumper.h b/clang/include/clang/AST/TextNodeDumper.h
index 1917a8ac29f05..1340fdb4acb51 100644
--- a/clang/include/clang/AST/TextNodeDumper.h
+++ b/clang/include/clang/AST/TextNodeDumper.h
@@ -9,6 +9,22 @@
// This file implements AST dumping of components of individual AST nodes.
//
//===----------------------------------------------------------------------===//
+//
+// Modifications to this file by SEI staff are copyright Carnegie Mellon
+// University and contributed under the Apache License v2.0 with LLVM
+// Exceptions.
+//
+// SEI Contributions are made with funding sand support from the Department of
+// Defense under Contract No. FA8702-15-D-0002 with Carnegie Mellon University
+// for the operation of the Software Engineering Institute, a federally funded
+// research and development center.
+//
+// The view, opinions, and/or findings contained in this material are those of
+// the author(s) and should not be construed as an official Government position,
+// policy, or decision, unless designated by other documentation.
+// DM24-0194
+//
+//===----------------------------------------------------------------------===//
#ifndef LLVM_CLANG_AST_TEXTNODEDUMPER_H
#define LLVM_CLANG_AST_TEXTNODEDUMPER_H
@@ -181,8 +197,17 @@ class TextNodeDumper
void Visit(QualType T);
+ // SEI: added to prevent this from being added whenever it comes from VarDecl
+ void Visit(QualType T, bool isFromDecl);
+
void Visit(TypeLoc);
+ // SEI: added support for getting ReturnType information
+ void VisitReturnType(QualType T);
+
+ // SEI: added support for more QT details. it's a passthrough for this class
+ void VisitQualTypeDetails(QualType T) {}
+
void Visit(const Decl *D);
void Visit(const CXXCtorInitializer *Init);
diff --git a/clang/lib/AST/JSONNodeDumper.cpp b/clang/lib/AST/JSONNodeDumper.cpp
index 64ddb1e739347..11cd21e844dba 100644
--- a/clang/lib/AST/JSONNodeDumper.cpp
+++ b/clang/lib/AST/JSONNodeDumper.cpp
@@ -70,6 +70,19 @@ void JSONNodeDumper::Visit(const Stmt *S) {
}
void JSONNodeDumper::Visit(const Type *T) {
+ // SEI: ensure FPTs are debloated. this can be expanded to ALL types, if
+ // desired
+ if (T && cacheAddress((void *)T)) {
+ // add this as a child to know that it's a Type
+ AddChild("typeDetails",
+ [=] { JOS.attribute("refId", createPointerRepresentation(T)); });
+
+ InnerTypeVisitor::Visit(T);
+ // SEI
+ VisitQualTypeDetails(T->getCanonicalTypeInternal());
+ return;
+ }
+
JOS.attribute("id", createPointerRepresentation(T));
if (!T)
@@ -86,15 +99,50 @@ void JSONNodeDumper::Visit(const Type *T) {
T->containsUnexpandedParameterPack());
attributeOnlyIfTrue("isImported", T->isFromAST());
InnerTypeVisitor::Visit(T);
+ // SEI
+ VisitQualTypeDetails(T->getCanonicalTypeInternal());
}
void JSONNodeDumper::Visit(QualType T) {
- JOS.attribute("id", createPointerRepresentation(T.getAsOpaquePtr()));
- JOS.attribute("kind", "QualType");
- JOS.attribute("type", createQualType(T));
- JOS.attribute("qualifiers", T.split().Quals.getAsString());
+
+ // SEI: used AddChild to prevent qualType from being part added to a list
+ // JOS.attributeArray("qualTypes", [=] {
+
+ // SEI: force qualType into its own block, otherwise multiple Visits
+ // create a bunch of siblings, which is invalid JSON
+ JOS.attributeBegin("qualType");
+ JOS.objectBegin();
+
+ // SEI: cache visited addresses and add only its refId
+ // instead of the kind, type, quals, but leave the qual type details
+ // because those can differ among IDs
+ if (cacheAddress(T.getAsOpaquePtr())) {
+ JOS.attribute("refId", createPointerRepresentation(T.getAsOpaquePtr()));
+ } else {
+ JOS.attribute("id", createPointerRepresentation(T.getAsOpaquePtr()));
+ JOS.attribute("kind", "QualType");
+ JOS.attribute("type", createQualType(T));
+ JOS.attribute("qualifiers", T.split().Quals.getAsString());
+ }
+
+ // SEI: get add'l info required for redemption analysis
+ // the qual type details differ even among cached references
+ VisitQualTypeDetails(T);
+
+ // SEI: if this is a pointer type, then recursively call ourselves
+ // until it's not
+ if (T->isPointerType())
+ Visit(T->getPointeeType());
+
+ JOS.objectEnd();
+ JOS.attributeEnd();
+ //} );
}
+// SEI: added this as a forwarding function so that the TextNodeDumper
+// SEI: doesn't have this applied when coming from a VarDecl
+void JSONNodeDumper::Visit(QualType T, bool isFromDecl) { Visit(T); }
+
void JSONNodeDumper::Visit(TypeLoc TL) {
if (TL.isNull())
return;
@@ -110,6 +158,64 @@ void JSONNodeDumper::Visit(TypeLoc TL) {
[TL, this] { writeSourceRange(TL.getSourceRange()); });
}
+void JSONNodeDumper::VisitQualTypeDetails(QualType T) {
+ // SEI: get more detailed info on type. this info is not transferrable
+ // with the refId, so this must be called on every type even if that type
+ // has been cached
+ JOS.attributeBegin("qualDetails");
+ JOS.arrayBegin();
+
+ auto CT = T->getCanonicalTypeInternal();
+
+ if (CT->isStructureType())
+ JOS.value("struct");
+
+ if (CT->isNullPtrType())
+ JOS.value("null");
+ if (CT->isUndeducedType())
+ JOS.value("undeduced");
+
+ if (CT->isPointerType())
+ JOS.value("ptr");
+ if (CT->isVoidType())
+ JOS.value("void");
+
+ if (CT->isSignedIntegerType())
+ JOS.value("signed");
+ if (CT->isUnsignedIntegerType())
+ JOS.value("unsigned");
+ if (CT->isIntegerType())
+ JOS.value("integer");
+ if (CT->isFloatingType())
+ JOS.value("fpp");
+ if (CT->isEnumeralType())
+ JOS.value("enum");
+ if (CT->isUnionType())
+ JOS.value("union");
+ if (CT->isFunctionPointerType())
+ JOS.value("func_ptr");
+ if (CT->isTypedefNameType())
+ JOS.value("type_def");
+ if (CT->isArrayType())
+ JOS.value("array");
+
+ JOS.arrayEnd();
+ JOS.attributeEnd();
+}
+
+// SEI: capture the return info in a nested JSON block
+void JSONNodeDumper::VisitReturnType(QualType T) {
+ // using this function allows us to easily wrap just the returnType
+ // section into its own JSON block. if we do this in ASTNodeTraverser,
+ // then the TextNodeDumper works as expected but the JSONNodeDumper
+ // rolls all siblings into the returnType node with those siblings as child
+ // nodes
+
+ JOS.attributeObject("returnTypeDetail", [=] { Visit(T); });
+
+ // Visit(T);
+}
+
void JSONNodeDumper::Visit(const Decl *D) {
JOS.attribute("id", createPointerRepresentation(D));
@@ -174,6 +280,14 @@ void JSONNodeDumper::Visit(const TemplateArgument &TA, SourceRange R,
void JSONNodeDumper::Visit(const CXXCtorInitializer *Init) {
JOS.attribute("kind", "CXXCtorInitializer");
+
+ // SEI: added id for
+ JOS.attribute("id", createPointerRepresentation(Init));
+
+ // SEI: added range for CXXCtorInitializers
+ JOS.attributeObject(
+ "range", [Init, this] { writeSourceRange(Init->getSourceRange()); });
+
if (Init->isAnyMemberInitializer())
JOS.attribute("anyInit", createBareDeclRef(Init->getAnyMember()));
else if (Init->isBaseInitializer())
@@ -957,6 +1071,10 @@ void JSONNodeDumper::VisitFieldDecl(const FieldDecl *FD) {
attributeOnlyIfTrue("modulePrivate", FD->isModulePrivate());
attributeOnlyIfTrue("isBitfield", FD->isBitField());
attributeOnlyIfTrue("hasInClassInitializer", FD->hasInClassInitializer());
+
+ // SEI: had to add this in b/c FieldDecls do not seem to call
+ // Visit(QualType)
+ Visit(FD->getType());
}
void JSONNodeDumper::VisitFunctionDecl(const FunctionDecl *FD) {
@@ -1350,6 +1468,8 @@ void JSONNodeDumper::VisitDeclRefExpr(const DeclRefExpr *DRE) {
case NOUR_Discarded: JOS.attribute("nonOdrUseReason", "discarded"); break;
}
attributeOnlyIfTrue("isImmediateEscalating", DRE->isImmediateEscalating());
+ // SEI: this doesn't call VisitNamedDecl, so we force it
+ Visit(DRE->getType());
}
void JSONNodeDumper::VisitSYCLUniqueStableNameExpr(
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index 6b524cfcd2d71..d295faa7f3442 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -242,6 +242,21 @@ void TextNodeDumper::Visit(QualType T) {
OS << " " << T.split().Quals.getAsString();
}
+// SEI: ignore this from being called whenever it's from a VarDecl
+// SEI: so we don't interfere with the normal text output
+void TextNodeDumper::Visit(QualType T, bool isFromDecl) {
+ if (!isFromDecl)
+ Visit(T);
+}
+
+void TextNodeDumper::VisitReturnType(QualType T) {
+ OS << "ReturnType";
+ dumpPointer(T.getAsOpaquePtr());
+ OS << " ";
+ dumpBareType(T, false);
+ OS << " " << T.split().Quals.getAsString();
+}
+
void TextNodeDumper::Visit(TypeLoc TL) {
if (!TL) {
ColorScope Color(OS, ShowColors, NullColor);
diff --git a/clang/test/AST/ByteCode/builtin-constant-p.cpp b/clang/test/AST/ByteCode/builtin-constant-p.cpp
index 315a907949c34..9f5521590833d 100644
--- a/clang/test/AST/ByteCode/builtin-constant-p.cpp
+++ b/clang/test/AST/ByteCode/builtin-constant-p.cpp
@@ -140,11 +140,3 @@ void test17(void) {
F("string literal" + 1); // both-warning {{adding}} \
// both-note {{use array indexing}}
}
-
-/// FIXME
-static void foo(int i) __attribute__((__diagnose_if__(!__builtin_constant_p(i), "not constant", "error"))) // expected-note {{from}}
-{
-}
-static void bar(int i) {
- foo(15); // expected-error {{not constant}}
-}
diff --git a/clang/test/AST/ByteCode/complex.cpp b/clang/test/AST/ByteCode/complex.cpp
index be10b3cfa53da..10ca82fbdc485 100644
--- a/clang/test/AST/ByteCode/complex.cpp
+++ b/clang/test/AST/ByteCode/complex.cpp
@@ -409,4 +409,4 @@ namespace ComplexConstexpr {
static_assert(__real test6 == 5, "");
static_assert(__imag test6 == 6, "");
static_assert(&__imag test6 == &__real test6 + 1, "");
-}
+}
\ No newline at end of file
diff --git a/clang/test/AST/ByteCode/const-eval.c b/clang/test/AST/ByteCode/const-eval.c
index 3e228226ac8c1..eab14c08ec809 100644
--- a/clang/test/AST/ByteCode/const-eval.c
+++ b/clang/test/AST/ByteCode/const-eval.c
@@ -180,9 +180,6 @@ typedef __INTPTR_TYPE__ intptr_t;
const intptr_t A = (intptr_t)(((int*) 0) + 1);
const intptr_t B = (intptr_t)(((char*)0) + 3);
_Static_assert(A > B, "");
-int * GH149500_p = &(*(int *)0x400);
-static const void *GH149500_q = &(*(const struct sysrq_key_op *)0);
-
#else
#error :(
#endif
diff --git a/clang/test/AST/ByteCode/cxx23.cpp b/clang/test/AST/ByteCode/cxx23.cpp
index 45dd4f528aefb..2856b872d44ab 100644
--- a/clang/test/AST/ByteCode/cxx23.cpp
+++ b/clang/test/AST/ByteCode/cxx23.cpp
@@ -369,26 +369,7 @@ namespace NestedUnions {
return true;
}
static_assert(test_nested());
-}
-
-namespace UnionMemberCallDiags {
- struct A { int n; };
- struct B { A a; };
- constexpr A a = (A() = B().a);
- union C {
- int n;
- A a;
- };
- constexpr bool g() {
- C c = {.n = 1};
- c.a.operator=(B{2}.a); // all-note {{member call on member 'a' of union with active member 'n' is not allowed in a constant expression}}
- return c.a.n == 2;
- }
- static_assert(g()); // all-error {{not an integral constant expression}} \
- // all-note {{in call to}}
}
-
-
#endif
diff --git a/clang/test/AST/ByteCode/placement-new.cpp b/clang/test/AST/ByteCode/placement-new.cpp
index b587cd6eaf89c..670def2d5870e 100644
--- a/clang/test/AST/ByteCode/placement-new.cpp
+++ b/clang/test/AST/ByteCode/placement-new.cpp
@@ -486,11 +486,3 @@ namespace bitcast {
}
static_assert(foo() == 0);
}
-
-constexpr int modify_const_variable() {
- const int a = 10;
- new ((int *)&a) int(12); // both-note {{modification of object of const-qualified type 'const int' is not allowed in a constant expression}}
- return a;
-}
-static_assert(modify_const_variable()); // both-error {{not an integral constant expression}} \
- // both-note {{in call to}}
diff --git a/clang/test/AST/ByteCode/records.cpp b/clang/test/AST/ByteCode/records.cpp
index 5ca3e2d12e2df..8925f2a2d0b67 100644
--- a/clang/test/AST/ByteCode/records.cpp
+++ b/clang/test/AST/ByteCode/records.cpp
@@ -1839,4 +1839,4 @@ namespace DiamondDowncast {
constexpr Top &top1 = (Middle1&)bottom;
constexpr Middle2 &fail = (Middle2&)top1; // both-error {{must be initialized by a constant expression}} \
// both-note {{cannot cast object of dynamic type 'const Bottom' to type 'Middle2'}}
-}
+}
\ No newline at end of file
diff --git a/clang/test/AST/HLSL/ByteAddressBuffers-AST.hlsl b/clang/test/AST/HLSL/ByteAddressBuffers-AST.hlsl
index f85194496942b..74fcfa6ac6129 100644
--- a/clang/test/AST/HLSL/ByteAddressBuff...
[truncated]
|
jrtc27
requested changes
Aug 6, 2025
There was a problem hiding this comment.
Having all these SEI comments isn't going to fly. Nor is deleting bits of existing tests. On top of that, if you're wanting to make significant changes to the AST, that may warrant an RFC. Not to mention that "one commit that does 7 enumerated things" is not a good way to seek review of things if it can be avoided.
AaronBallman
reviewed
Aug 7, 2025
There was a problem hiding this comment.
In addition to the comments from @jrtc27, the copyright notices may also be problematic (I commented in the PR on it).
I realize you've got constraints on your end that may make this impractical, but from our end what we'd like to see is:
- The SEI comments should be removed, probably including the copyright notices
- Any existing test code that you've removed needs to be restored; it's fine to modify tests to expect new output (that's normal) but it's generally not okay to remove existing test coverage without justifying why it's safe to remove
- Splitting the PR up into multiple logical changes so it's less massive
Comment on lines
+15
to
+17
| // Modifications to this file by SEI staff are copyright Carnegie Mellon | ||
| // University and contributed under the Apache License v2.0 with LLVM | ||
| // Exceptions. |
There was a problem hiding this comment.
CC @llvm-beanz regarding licensing concerns. Pretty sure this is not something we can accept per: https://llvm.org/docs/DeveloperPolicy.html#embedded-copyright-or-contributed-by-statements
There was a problem hiding this comment.
Confirmed. We do not accept embedded copyright notices.
| // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}} <col:18> col:23 'float' | ||
| // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}} <col:25, col:31> col:29 'int' cinit | ||
| // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}} <col:31> 'int' 12 | ||
| // CHECK: ParmVarDecl 0x{{[^ ]*}} <col:25, col:31> col:29 'int' cinit |
There was a problem hiding this comment.
We're prefer to keep the CHECK-NEXT lines whenever possible because otherwise the test failures become harder to reason about when someone breaks the functionality (it also makes it easier to hide test failures).
Comment on lines
+9
to
+13
| // CHECK-NEXT: `-typeDetails: ParenType {{.*}} | ||
| // CHECK-NEXT: `-typeDetails: FunctionProtoType {{.*}} 'void () noexcept(10 > 5)' exceptionspec_noexcept_true cdecl | ||
| // CHECK-NEXT: |-NoexceptExpr: ConstantExpr {{.*}} 'bool' | ||
| // CHECK-NEXT: | `-value: Int 1 | ||
| // CHECK-NEXT: `-BuiltinType {{.*}} 'void' | ||
| // CHECK: `-functionDetails: cdeclReturnType {{.*}} 'void' |
There was a problem hiding this comment.
Can you explain a bit about how this helps folks with the textual dumps? It looks like this is more for folks trying to script the results, but those uses should be using the JSON dump output because that's actually structured output. The text node dumper is mostly used by folks who are in a debugger and want more information about what's being debugged, and for them these extra "details" bits are likely to be noise.
There was a problem hiding this comment.
We tried to leave the text dumps alone as much as possible. Unfortunately, to get some of the additional information that we needed, we had to update ASTNodeTraverser. Those changes mean that TextNodeDumper now receives the additional information as well.
There was a problem hiding this comment.
I agree the changes to the Textual AST dump don't seem helpful for its intended purpose.
You could make your changes to the JSON AST node dumper only. Even then, a lot of the new information looks like it wouldn't be very interesting to most users and is quite extra verbose, so it could be hidden behind a flag.
mizvekov
reviewed
Aug 13, 2025
| // University and contributed under the Apache License v2.0 with LLVM | ||
| // Exceptions. | ||
| // | ||
| // SEI Contributions are made with funding sand support from the Department of |
There was a problem hiding this comment.
Suggested change
| // SEI Contributions are made with funding sand support from the Department of | |
| // SEI Contributions are made with funding and support from the Department of |
There was a problem hiding this comment.
Typo, but we probably can't allow this copyright notice anyway, per project policy.
Comment on lines
+9
to
+13
| // CHECK-NEXT: `-typeDetails: ParenType {{.*}} | ||
| // CHECK-NEXT: `-typeDetails: FunctionProtoType {{.*}} 'void () noexcept(10 > 5)' exceptionspec_noexcept_true cdecl | ||
| // CHECK-NEXT: |-NoexceptExpr: ConstantExpr {{.*}} 'bool' | ||
| // CHECK-NEXT: | `-value: Int 1 | ||
| // CHECK-NEXT: `-BuiltinType {{.*}} 'void' | ||
| // CHECK: `-functionDetails: cdeclReturnType {{.*}} 'void' |
There was a problem hiding this comment.
I agree the changes to the Textual AST dump don't seem helpful for its intended purpose.
You could make your changes to the JSON AST node dumper only. Even then, a lot of the new information looks like it wouldn't be very interesting to most users and is quite extra verbose, so it could be hidden behind a flag.
tbaederr
removed
the
clang:bytecode
|
This seems large enough that it might warrant an RFC with a more detailed motivation |
I think I agree with Aaron's feedback back in August, all those points seem valid. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
AST output modifications primarily focused on JSON enhancements for the SEI Redemption project. Some of the key changes are:
const,volatile, and othersptr,signed,float,struct,union,array,promotable,integer,func_ptrrefIDkey for referring back to the original IDExamples